changelog of trunk including comments / devtalk

srsa_4c

4451

Re: changelog of trunk including comments / devtalk

« Reply #280 on: 21 / May / 2019, 13:25:22 »

Advertisements

Compiling current trunk (r5210...5214) gives me this:

Code: [Select]

ptp_op_names.c:243:12: error: 'NULL' undeclared (first use in this function)
     return NULL;
            ^

My host compiler is

Code: [Select]

gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-suse-linux/4.8/lto-wrapper
Target: x86_64-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,java,ada --enable-checking=release --with-gxx-include-dir=/usr/include/c++/4.8 --enable-ssp --disable-libssp --disable-plugin --with-bugurl=http://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --disable-libgcj --disable-libmudflap --with-slibdir=/lib64 --with-system-zlib --enable-__cxa_atexit --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --enable-linker-build-id --enable-linux-futex --program-suffix=-4.8 --without-system-libunwind --with-arch-32=i586 --with-tune=generic --build=x86_64-suse-linux --host=x86_64-suse-linux
Thread model: posix
gcc version 4.8.5 (SUSE Linux)

Apparently, no such problems on the autobuild and at the other devs. What could I be missing?

Logged

reyalp

13880

Re: changelog of trunk including comments / devtalk

« Reply #281 on: 21 / May / 2019, 13:40:09 »

Quote from: srsa_4c on 21 / May / 2019, 13:25:22

Apparently, no such problems on the autobuild and at the other devs. What could I be missing?

I added stddef which should ensure it's defined. Not including it was a mistake, which I didn't notice because apparently NULL gets implicitly defined by stdint on some compilers.

My host compiler is

Code: [Select]

$ gcc -v
Using built-in specs.
COLLECT_GCC=C:\msys64\mingw32\bin\gcc.exe
COLLECT_LTO_WRAPPER=C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.4.0/lto-wrapper.exe
Target: i686-w64-mingw32
Configured with: ../gcc-7.4.0/configure --prefix=/mingw32 --with-local-prefix=/mingw32/local --build=i686-w64-mingw32 --host=i686-w64-mingw32 --target=i686-w64-mingw32 --with-native-system-header-dir=/mingw32/i686-w64-mingw32/include --libexecdir=/mingw32/lib --enable-bootstrap --with-arch=i686 --with-tune=generic --enable-languages=c,lto,c++,objc,obj-c++,fortran,ada --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-time=yes --enable-libstdcxx-filesystem-ts=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw32 --with-mpfr=/mingw32 --with-mpc=/mingw32 --with-isl=/mingw32 --with-pkgversion='Rev1, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld --disable-sjlj-exceptions --with-dwarf2
Thread model: posix
gcc version 7.4.0 (Rev1, Built by MSYS2 project)

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: changelog of trunk including comments / devtalk

« Reply #282 on: 21 / May / 2019, 14:07:41 »

Quote from: reyalp on 21 / May / 2019, 13:40:09

I added stddef which should ensure it's defined.

Thanks.

Quote

Not including it was a mistake, which I didn't notice because apparently NULL gets implicitly defined by stdint on some compilers.

Since this was the second time such things happened to me, I looked up the cause:
Standards.html packaged with gcc 4 states that

Quote

The default, if no C language dialect options are given,
is -std=gnu90; this is intended to change to -std=gnu11 in some future release.

Current gcc docs say that

Quote

The default, if no C language dialect options are given, is -std=gnu11.

Maybe we should specify -std in our makefiles?

Logged

reyalp

13880

Re: changelog of trunk including comments / devtalk

« Reply #283 on: 22 / May / 2019, 02:53:19 »

Quote from: srsa_4c on 21 / May / 2019, 14:07:41

Maybe we should specify -std in our makefiles?

That seems like a good idea in general, but in my environment at least setting -std=gnu90 or -std=gnu11 makes to no difference for the NULL issue. The msys stdint.h appears to define NULL through one of things it includes.

I don't think NULL should be defined implicitly by any version of the C standard, but I could be wrong.

« Last Edit: 22 / May / 2019, 18:10:04 by reyalp »

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: changelog of trunk including comments / devtalk

« Reply #284 on: 22 / May / 2019, 14:51:04 »

Quote from: reyalp on 22 / May / 2019, 02:53:19

That seems like a good idea in general, but in my environment at least setting -std=gnu90 or -std=gnu11 makes to difference for the NULL issue. The msys stdint.h appears to define NULL through one of things it includes.

I don't think NULL should be defined implicitly by any version of the C standard, but I could be wrong.

I tried building r5214 with -std=gnu11 added to the host CFLAGS, and it worked.
Which standard do we want? The code has been kept compatible with gnu90 so far.

Logged

reyalp

13880

Re: changelog of trunk including comments / devtalk

« Reply #285 on: 22 / May / 2019, 18:09:35 »

Quote from: srsa_4c on 22 / May / 2019, 14:51:04

Which standard do we want? The code has been kept compatible with gnu90 so far.

I don't have a strong preference. We're not likely to need the newer features of c11, but some of them could be convenient. From a quick look, it appears that GCC 4.7 added the gnu11 option in 2012 (https://www.gnu.org/software/gcc/gcc-4.7/changes.html) so if people are still building with earlier versions, that would be a reason to use the older one. Most vaguely current distros should be on something newer, even RHEL7 is on 4.8.
I'm not sure what the host compiler on the autobuild is now, but it should be at least 4.7.

We could always switch to a newer standard later.

Setting the gnu90 won't avoid recurrence of the particular problem with NULL, since my setup compiled the offending file with either standard.

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: changelog of trunk including comments / devtalk

« Reply #286 on: 23 / May / 2019, 12:18:09 »

Quote from: reyalp on 26 / April / 2019, 01:12:42

I compared output of the unpatched version with the patched version, with both capstone 3 and 4. No difference for any model I have dumps for. I also did full disassembly of g7x and sx710, and got effectively identical output. On sx710, a few literal pools where garbage previously disassembled as ssat/usat failed to disassemble instead. All other differences were due CHDK code/stubs changes since the last time I did a full disassembly.

IMO, we should drop capstone 3 support fairly quickly, since it requires a source patch and there's a risk of getting different output from things like the differences in handling invalid code.

I was using a patched 3.0.4 capstone library, up until today. When rebuilding stubs for thumb2 cameras, I noticed that the results differ from official source. Diff attached (a bunch of ptp handlers not identified). I then removed the 3.0.4 files, installed 4.0.1, and got no differences when rebuilding stubs.

t2_csvs_w_capstone3.diff (98.6 kB - downloaded 25 times.)

Logged

reyalp

13880

Re: changelog of trunk including comments / devtalk

« Reply #287 on: 23 / May / 2019, 13:27:45 »

Quote from: srsa_4c on 23 / May / 2019, 12:18:09

I was using a patched 3.0.4 capstone library, up until today. When rebuilding stubs for thumb2 cameras, I noticed that the results differ from official source. Diff attached (a bunch of ptp handlers not identified). I then removed the 3.0.4 files, installed 4.0.1, and got no differences when rebuilding stubs.

Thanks for catching that. I'll take a look later, but this is why I'd prefer to standardize on 4.x. I don't want to routinely test multiple versions or maintain all the ifdefs.

In practice, I think this means making sure the windows tool https://chdk.setepontos.com/index.php?topic=12752.0 plays nicely with it. The issue is that the precompiled binaries only work with gcc if linked to the DLL, and then the DLL needs to be on the path, and the library name changes. So we need to either provide a static package (which should be pretty easy) or adjust what options the tool uses. I'll try to put together a static, GCC compiled package that works with the windows toolchain.

Once that's done and posted, I'll probably change the thumb2 tools to error if capstone is the older version, so people know to update.

Logged

Don't forget what the H stands for.

reyalp

13880

Re: changelog of trunk including comments / devtalk

« Reply #288 on: 25 / May / 2019, 03:08:57 »

Quote from: srsa_4c on 23 / May / 2019, 12:18:09

I was using a patched 3.0.4 capstone library, up until today. When rebuilding stubs for thumb2 cameras, I noticed that the results differ from official source. Diff attached (a bunch of ptp handlers not identified). I then removed the 3.0.4 files, installed 4.0.1, and got no differences when rebuilding stubs.

It looks like a bug in capstone 3.x, arm.operands[1].shift.value is not set. The check doesn't seem to be required for any current firmware, so I commented it out.

Logged

Don't forget what the H stands for.

reyalp

13880

Re: changelog of trunk including comments / devtalk

« Reply #289 on: 16 / May / 2020, 23:47:29 »

In r5495, I added a camera.h define CAM_ILC for the EOS M cams. This allows things like the zoom functions and defines to be handled more cleanly. Initial motivation was related to addressing CAM_USE_ALT_PT_MoveOpticalZoomAt default (https://chdk.setepontos.com/index.php?topic=13875.msg142050#msg142050) but it may be useful elsewhere.

shooting_set_zoom*, lens_set_zoom_point and lens_set_zoom_speed are now noop for CAM_ILC.

I haven't added it to camera_info and script, but might later.

Logged

Don't forget what the H stands for.