Skip to content

remove "-g" (include debug flags) compiler option #305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
sadielbartholomew merged 1 commit into from
Feb 18, 2022
Merged

remove "-g" (include debug flags) compiler option #305

sadielbartholomew merged 1 commit into from
Feb 18, 2022

Conversation

@alaniwi
Copy link
Contributor

@alaniwi alaniwi commented Jan 31, 2022

This works around what seems to be some kind of compiler / linker bug.

In the conda environment we are using, containing:

$ gcc --version
gcc (GCC) 11.2.0
[...]

$ ld --version
GNU ld version 2.27-44.base.el7
[...]

I find that I can't create a shared library if -g has been used in the compilation:

$ cat foo.c
void foo() {}

$ gcc -c -g foo.c
$ ld -shared -o foo.so foo.o
ld: foo.o: unable to initialize decompress status for section .debug_info
ld: foo.o: unable to initialize decompress status for section .debug_info
foo.o: file not recognized: File format not recognized

without the -g, everything looks normal

$ rm foo.o
$ gcc -c foo.c
$ ld -shared -o foo.so foo.o
$ nm foo.so
0000000000201000 D __bss_start
0000000000200f50 d _DYNAMIC
0000000000201000 D _edata
0000000000201000 D _end
000000000000024d T foo
0000000000201000 d _GLOBAL_OFFSET_TABLE_

Hence removing -g from the compiler options to work around this.

@alaniwi
remove "-g" (include debug flags) compiler option
16525ea 
Works around what seems to be some kind of linker bug.
@alaniwi
Copy link
Contributor Author

alaniwi commented Jan 31, 2022

@davidhassell It turns out the problem was that the ld was from outside the conda enviroment, whereas the gcc was from within the conda environment, and the two were incompatible. We can address that separately. Nonetheless, the -g should not be necessary for use in cf-python, so I think that this pull request still stands.

@alaniwi alaniwi mentioned this pull request Jan 31, 2022
Closed
@sadielbartholomew
Copy link
Member

sadielbartholomew commented Feb 18, 2022 •
edited
Loading

Thanks for this PR Alan, and sorry for the delayed reply, myself and @davidhassell have been very busy lately. David is now on leave, so I'll review this now.

FYI, I set the CI jobs to run, the failure in the 'Run test suite / test-suite-job-0' which cancelled the other jobs is due to a flaky test which is obviously unrelated to your Makefile change comprising this PR. The important thing is that I can see from the logs that in those jobs (pre-cancellation for all but that one) the 'Make UMRead' step ran fine, e.g:

$ cd /Users/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib
$ make
gcc -Wall -fPIC -DLINUX -c umfile.c
gcc -Wall -fPIC -DLINUX -c error.c
gcc -Wall -fPIC -DLINUX -c filetype.c
gcc -Wall -fPIC -DLINUX -c malloc.c
gcc -Wall -fPIC -DLINUX -c linklist.c
gcc -Wall -fPIC -DLINUX -c new_structs.c
gcc -Wall -fPIC -DLINUX -c swap.c
make -C type-dep
make[1]: Entering directory '/home/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib/type-dep'
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o umfile_test_typedep_sgl.o umfile_test_typedep.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o interpret_header_sgl.o interpret_header.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o read_sgl.o read.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o process_vars_sgl.o process_vars.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o debug_dump_sgl.o debug_dump.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o date_and_time_sgl.o date_and_time.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o compare_sgl.o compare.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o levels_sgl.o levels.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o axes_sgl.o axes.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o unwgdos_sgl.o unwgdos.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o umfile_test_typedep_dbl.o umfile_test_typedep.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o interpret_header_dbl.o interpret_header.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o read_dbl.o read.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o process_vars_dbl.o process_vars.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o debug_dump_dbl.o debug_dump.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o date_and_time_dbl.o date_and_time.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o compare_dbl.o compare.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o levels_dbl.o levels.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o axes_dbl.o axes.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o unwgdos_dbl.o unwgdos.c
rm -f umfile_typedep.a
ar r umfile_typedep.a  umfile_test_typedep_sgl.o  interpret_header_sgl.o  read_sgl.o  process_vars_sgl.o  debug_dump_sgl.o  date_and_time_sgl.o  compare_sgl.o  levels_sgl.o  axes_sgl.o  unwgdos_sgl.o  umfile_test_typedep_dbl.o  interpret_header_dbl.o  read_dbl.o  process_vars_dbl.o  debug_dump_dbl.o  date_and_time_dbl.o  compare_dbl.o  levels_dbl.o  axes_dbl.o  unwgdos_dbl.o
ar: creating umfile_typedep.a
make[1]: Leaving directory '/home/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib/type-dep'
ld -shared --build-id -o umfile.so umfile.o error.o filetype.o malloc.o linklist.o new_structs.o swap.o --whole-archive type-dep/umfile_typedep.a

so the removal of the -g flag seems to be fine across those cases, including on MacOS:

$ cd /Users/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib
$ make
clang -Wall -fPIC -DOSX -c umfile.c
clang -Wall -fPIC -DOSX -c error.c
clang -Wall -fPIC -DOSX -c filetype.c
clang -Wall -fPIC -DOSX -c malloc.c
clang -Wall -fPIC -DOSX -c linklist.c
clang -Wall -fPIC -DOSX -c new_structs.c
clang -Wall -fPIC -DOSX -c swap.c
/Applications/Xcode_13.2.1.app/Contents/Developer/usr/bin/make -C type-dep
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o umfile_test_typedep_sgl.o umfile_test_typedep.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o interpret_header_sgl.o interpret_header.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o read_sgl.o read.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o process_vars_sgl.o process_vars.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o debug_dump_sgl.o debug_dump.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o date_and_time_sgl.o date_and_time.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o compare_sgl.o compare.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o levels_sgl.o levels.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o axes_sgl.o axes.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o unwgdos_sgl.o unwgdos.c
unwgdos.c:371:45: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
      *inum = (*icomp << (31-istart)) & (~0 << 31);
                                         ~~ ^
unwgdos.c:3[9](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:9)6:[27](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:27): warning: shifting a negative signed value is undefined [-Wshift-negative-value]
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o umfile_test_typedep_dbl.o umfile_test_typedep.c
  i = (ui >> ibit) & ~(~0 << 1);
                       ~~ ^
2 warnings generated.
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o interpret_header_dbl.o interpret_header.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o read_dbl.o read.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o process_vars_dbl.o process_vars.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o debug_dump_dbl.o debug_dump.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o date_and_time_dbl.o date_and_time.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o compare_dbl.o compare.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o levels_dbl.o levels.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o axes_dbl.o axes.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o unwgdos_dbl.o unwgdos.c
unwgdos.c:371:45: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
      *inum = (*icomp << ([31](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:31)-istart)) & (~0 << 31);
                                         ~~ ^
rm -f umfile_typedep.a
unwgdos.c:[39](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:39)6:27: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
ar r umfile_typedep.a  umfile_test_typedep_sgl.o  interpret_header_sgl.o  read_sgl.o  process_vars_sgl.o  debug_dump_sgl.o  date_and_time_sgl.o  compare_sgl.o  levels_sgl.o  axes_sgl.o  unwgdos_sgl.o  umfile_test_typedep_dbl.o  interpret_header_dbl.o  read_dbl.o  process_vars_dbl.o  debug_dump_dbl.o  date_and_time_dbl.o  compare_dbl.o  levels_dbl.o  axes_dbl.o  unwgdos_dbl.o
  i = (ui >> ibit) & ~(~0 << 1);
                       ~~ ^
2 warnings generated.
ar: creating archive umfile_typedep.a
clang -dynamiclib -o umfile.so umfile.o error.o filetype.o malloc.o linklist.o new_structs.o swap.o -force_load type-dep/umfile_typedep.a

(Though it works and builds fine, I see there is a warning raised in the above make output, which I will raise as a separate Issue and tag you to check if you wouldn't mind, since I know little C and know you wrote the C for cf-python.)

I'm testing your PR locally now and will submit a review in a moment.

sadielbartholomew
sadielbartholomew approved these changes Feb 18, 2022
View reviewed changes
Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, thank you Alan (we can ignore the failing CI job for reasons commented previously).

@sadielbartholomew sadielbartholomew merged commit 193211a into NCAS-CMS:master Feb 18, 2022
@davidhassell davidhassell added this to the 3.13.0 milestone Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sadielbartholomew sadielbartholomew sadielbartholomew approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.13.0

Development

Successfully merging this pull request may close these issues.

3 participants

@alaniwi @sadielbartholomew @davidhassell