Re: [stage1][PATCH] Change semantics of -frecord-gcc-switches and add -frecord-gcc-switches-format.
On 3/18/20 10:05 AM, Martin Liška wrote: On 3/17/20 7:43 PM, Egeyar Bagcioglu wrote: Hi Martin, I like the patch. It definitely serves our purposes at Oracle and provides another way to do what my previous patches did as well. 1) It keeps the backwards compatibility regarding -frecord-gcc-switches; therefore, removes my related doubts about your previous patch. 2) It still makes use of -frecord-gcc-switches. The new option is only to control the format. This addresses some previous objections to having a new option doing something similar. Now the new option controls the behaviour of the existing one and that behaviour can be further extended. 3) It uses an environment variable as Jakub suggested. The patch looks good and I confirm that it works for our purposes. Hello. Thank you for the support. Having said that, I have to ask for recognition in this patch for my and my company's contributions. Can you please keep my name and my work email in the changelog and in the commit message? Sure, sorry I forgot. Hi Martin, I noticed that some comments in the patch were still referring to --record-gcc-command-line, the option I suggested earlier. I updated those comments to mention -frecord-gcc-switches-format instead and also added my name to the patch as you agreed above. I attached the updated patch. We are starting to use this patch in the specific domain where we need its functionality. Regards Egeyar Martin Thanks Egeyar On 3/17/20 2:53 PM, Martin Liška wrote: Hi. I'm sending enhanced patch that makes the following changes: - a new option -frecord-gcc-switches-format is added; the option selects format (processed, driver) for all options that record GCC command line - Dwarf gen_produce_string is now used in -fverbose-asm - The .s file is affected in the following way: BEFORE: # GNU C17 (SUSE Linux) version 9.2.1 20200128 [revision 83f65674e78d97d27537361de1a9d74067ff228d] (x86_64-suse-linux) # compiled by GNU C version 9.2.1 20200128 [revision 83f65674e78d97d27537361de1a9d74067ff228d], GMP version 6.2.0, MPFR version 4.0.2, MPC version 1.1.0, isl version isl-0.22.1-GMP # GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 # options passed: -fpreprocessed test.i -march=znver1 -mmmx -mno-3dnow # -msse -msse2 -msse3 -mssse3 -msse4a -mcx16 -msahf -mmovbe -maes -msha # -mpclmul -mpopcnt -mabm -mno-lwp -mfma -mno-fma4 -mno-xop -mbmi -mno-sgx # -mbmi2 -mno-pconfig -mno-wbnoinvd -mno-tbm -mavx -mavx2 -msse4.2 -msse4.1 # -mlzcnt -mno-rtm -mno-hle -mrdrnd -mf16c -mfsgsbase -mrdseed -mprfchw # -madx -mfxsr -mxsave -mxsaveopt -mno-avx512f -mno-avx512er -mno-avx512cd # -mno-avx512pf -mno-prefetchwt1 -mclflushopt -mxsavec -mxsaves # -mno-avx512dq -mno-avx512bw -mno-avx512vl -mno-avx512ifma -mno-avx512vbmi # -mno-avx5124fmaps -mno-avx5124vnniw -mno-clwb -mmwaitx -mclzero -mno-pku # -mno-rdpid -mno-gfni -mno-shstk -mno-avx512vbmi2 -mno-avx512vnni # -mno-vaes -mno-vpclmulqdq -mno-avx512bitalg -mno-movdiri -mno-movdir64b # -mno-waitpkg -mno-cldemote -mno-ptwrite --param l1-cache-size=32 # --param l1-cache-line-size=64 --param l2-cache-size=512 -mtune=znver1 # -grecord-gcc-switches -g -fverbose-asm -frecord-gcc-switches # options enabled: -faggressive-loop-optimizations -fassume-phsa # -fasynchronous-unwind-tables -fauto-inc-dec -fcommon # -fdelete-null-pointer-checks -fdwarf2-cfi-asm -fearly-inlining # -feliminate-unused-debug-types -ffp-int-builtin-inexact -ffunction-cse # -fgcse-lm -fgnu-runtime -fgnu-unique -fident -finline-atomics # -fipa-stack-alignment -fira-hoist-pressure -fira-share-save-slots # -fira-share-spill-slots -fivopts -fkeep-static-consts # -fleading-underscore -flifetime-dse -flto-odr-type-merging -fmath-errno # -fmerge-debug-strings -fpeephole -fplt -fprefetch-loop-arrays # -frecord-gcc-switches -freg-struct-return -fsched-critical-path-heuristic # -fsched-dep-count-heuristic -fsched-group-heuristic -fsched-interblock # -fsched-last-insn-heuristic -fsched-rank-heuristic -fsched-spec # -fsched-spec-insn-heuristic -fsched-stalled-insns-dep -fschedule-fusion # -fsemantic-interposition -fshow-column -fshrink-wrap-separate # -fsigned-zeros -fsplit-ivs-in-unroller -fssa-backprop -fstdarg-opt # -fstrict-volatile-bitfields -fsync-libcalls -ftrapping-math -ftree-cselim # -ftree-forwprop -ftree-loop-if-convert -ftree-loop-im -ftree-loop-ivcanon # -ftree-loop-optimize -ftree-parallelize-loops= -ftree-phiprop # -ftree-reassoc -ftree-scev-cprop -funit-at-a-time -funwind-tables # -fverbose-asm -fzero-initialized-in-bss -m128bit-long-double -m64 -m80387 # -mabm -madx -maes -malign-stringops -mavx -mavx2 # -mavx256-split-unaligned-store -mbmi -mbmi2 -mclflushopt -mclzero -mcx16 # -mf16c -mfancy-math-387 -mfma -mfp-ret-in-387 -mfsgsbase -mfxsr -mglibc # -mieee-fp -mlong-double-80 -mlzcnt -mmmx -mmovbe -mmwaitx -mpclmul # -mpopcnt -mprfchw -mpush-args -mrdrnd -mrdseed -mred-zone -msahf -msha # -msse
Re: [stage1][PATCH] Change semantics of -frecord-gcc-switches and add -frecord-gcc-switches-format.
Hi Martin, I like the patch. It definitely serves our purposes at Oracle and provides another way to do what my previous patches did as well. 1) It keeps the backwards compatibility regarding -frecord-gcc-switches; therefore, removes my related doubts about your previous patch. 2) It still makes use of -frecord-gcc-switches. The new option is only to control the format. This addresses some previous objections to having a new option doing something similar. Now the new option controls the behaviour of the existing one and that behaviour can be further extended. 3) It uses an environment variable as Jakub suggested. The patch looks good and I confirm that it works for our purposes. Having said that, I have to ask for recognition in this patch for my and my company's contributions. Can you please keep my name and my work email in the changelog and in the commit message? Thanks Egeyar On 3/17/20 2:53 PM, Martin Liška wrote: Hi. I'm sending enhanced patch that makes the following changes: - a new option -frecord-gcc-switches-format is added; the option selects format (processed, driver) for all options that record GCC command line - Dwarf gen_produce_string is now used in -fverbose-asm - The .s file is affected in the following way: BEFORE: # GNU C17 (SUSE Linux) version 9.2.1 20200128 [revision 83f65674e78d97d27537361de1a9d74067ff228d] (x86_64-suse-linux) # compiled by GNU C version 9.2.1 20200128 [revision 83f65674e78d97d27537361de1a9d74067ff228d], GMP version 6.2.0, MPFR version 4.0.2, MPC version 1.1.0, isl version isl-0.22.1-GMP # GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 # options passed: -fpreprocessed test.i -march=znver1 -mmmx -mno-3dnow # -msse -msse2 -msse3 -mssse3 -msse4a -mcx16 -msahf -mmovbe -maes -msha # -mpclmul -mpopcnt -mabm -mno-lwp -mfma -mno-fma4 -mno-xop -mbmi -mno-sgx # -mbmi2 -mno-pconfig -mno-wbnoinvd -mno-tbm -mavx -mavx2 -msse4.2 -msse4.1 # -mlzcnt -mno-rtm -mno-hle -mrdrnd -mf16c -mfsgsbase -mrdseed -mprfchw # -madx -mfxsr -mxsave -mxsaveopt -mno-avx512f -mno-avx512er -mno-avx512cd # -mno-avx512pf -mno-prefetchwt1 -mclflushopt -mxsavec -mxsaves # -mno-avx512dq -mno-avx512bw -mno-avx512vl -mno-avx512ifma -mno-avx512vbmi # -mno-avx5124fmaps -mno-avx5124vnniw -mno-clwb -mmwaitx -mclzero -mno-pku # -mno-rdpid -mno-gfni -mno-shstk -mno-avx512vbmi2 -mno-avx512vnni # -mno-vaes -mno-vpclmulqdq -mno-avx512bitalg -mno-movdiri -mno-movdir64b # -mno-waitpkg -mno-cldemote -mno-ptwrite --param l1-cache-size=32 # --param l1-cache-line-size=64 --param l2-cache-size=512 -mtune=znver1 # -grecord-gcc-switches -g -fverbose-asm -frecord-gcc-switches # options enabled: -faggressive-loop-optimizations -fassume-phsa # -fasynchronous-unwind-tables -fauto-inc-dec -fcommon # -fdelete-null-pointer-checks -fdwarf2-cfi-asm -fearly-inlining # -feliminate-unused-debug-types -ffp-int-builtin-inexact -ffunction-cse # -fgcse-lm -fgnu-runtime -fgnu-unique -fident -finline-atomics # -fipa-stack-alignment -fira-hoist-pressure -fira-share-save-slots # -fira-share-spill-slots -fivopts -fkeep-static-consts # -fleading-underscore -flifetime-dse -flto-odr-type-merging -fmath-errno # -fmerge-debug-strings -fpeephole -fplt -fprefetch-loop-arrays # -frecord-gcc-switches -freg-struct-return -fsched-critical-path-heuristic # -fsched-dep-count-heuristic -fsched-group-heuristic -fsched-interblock # -fsched-last-insn-heuristic -fsched-rank-heuristic -fsched-spec # -fsched-spec-insn-heuristic -fsched-stalled-insns-dep -fschedule-fusion # -fsemantic-interposition -fshow-column -fshrink-wrap-separate # -fsigned-zeros -fsplit-ivs-in-unroller -fssa-backprop -fstdarg-opt # -fstrict-volatile-bitfields -fsync-libcalls -ftrapping-math -ftree-cselim # -ftree-forwprop -ftree-loop-if-convert -ftree-loop-im -ftree-loop-ivcanon # -ftree-loop-optimize -ftree-parallelize-loops= -ftree-phiprop # -ftree-reassoc -ftree-scev-cprop -funit-at-a-time -funwind-tables # -fverbose-asm -fzero-initialized-in-bss -m128bit-long-double -m64 -m80387 # -mabm -madx -maes -malign-stringops -mavx -mavx2 # -mavx256-split-unaligned-store -mbmi -mbmi2 -mclflushopt -mclzero -mcx16 # -mf16c -mfancy-math-387 -mfma -mfp-ret-in-387 -mfsgsbase -mfxsr -mglibc # -mieee-fp -mlong-double-80 -mlzcnt -mmmx -mmovbe -mmwaitx -mpclmul # -mpopcnt -mprfchw -mpush-args -mrdrnd -mrdseed -mred-zone -msahf -msha # -msse -msse2 -msse3 -msse4 -msse4.1 -msse4.2 -msse4a -mssse3 -mstv # -mtls-direct-seg-refs -mvzeroupper -mxsave -mxsavec -mxsaveopt -mxsaves AFTER: # GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 # GNU C17 10.0.1 20200317 (experimental) -march=znver1 -mmmx -mno-3dnow -msse -msse2 -msse3 -mssse3 -msse4a -mcx16 -msahf -mmovbe -maes -msha -mpclmul -mpopcnt -mabm -mno-lwp -mfma -mno-fma4 -mno-xop -mbmi -mno-sgx -mbmi2 -mno-pconfig -mno-wbnoinvd -mno-tbm -mavx -mavx2 -msse4.2 -msse4.1 -mlzcnt -mno-rtm -mno-hle -mrdrnd -mf16c -mfsgsbase -mrdseed -mprfchw -madx
Re: [PATCH v2 0/3] Introduce a new GCC option, --record-gcc-command-line
On 3/5/20 8:36 AM, Richard Biener wrote: On Wed, Mar 4, 2020 at 5:28 PM Egeyar Bagcioglu wrote: On 3/4/20 1:18 AM, Fangrui Song wrote: On 2020-03-03, Joseph Myers wrote: On Tue, 3 Mar 2020, Egeyar Bagcioglu wrote: Although we discussed after the submission of the first version that there are several other options performing similar tasks, I believe we established that there is still a need for this specific functionality. Therefore, I am skipping in this email the comparison between this option and the existing options with similarities. Mentioning -frecord-gcc-switches will be much appreciated. How is the new .GCC.command.line different? Does it still have the SHF_MERGE | SHF_STRINGS flag? If you change the flags, the .GCC.command.line section may not play with another object file (generated by -frecord-gcc-switches) whose .GCC.command.line is SHF_MERGE | SHF_STRINGS. When both -frecord-gcc-switches and --record-command-line are specified, is it an error? This option is similar to -frecord-gcc-switches. However, they have three fundamental differences: Firstly, -frecord-gcc-switches saves the internal state after the argv is processed and passed by the driver. As opposed to that, --record-gcc-command-line saves the command-line as received by the driver, with the exception of extending @files first. Secondly, -frecord-gcc-switches saves the switches as separate entries into a mergeable string section. Therefore, the entries belonging to different object files get mixed up after being linked. The new --record-gcc-command-line, on the other hand, creates one entry per invocation. By doing so, it makes it clear which options were used together in a single gcc invocation. Lastly, --record-gcc-command-line also adds the version of the gcc into this single entry to make it clear which version of gcc was called with any given command line. This is useful in cases where .comment section reports multiple versions. While there are also similarities between the implementations of these two options, those implementations are completely independent. These commands can be used separately or together without issues. I used the same section that -frecord-gcc-switches uses on purpose, so that they can also be used together to save both the command line given to GCC and the internal switches passed by GCC. The option -grecord-gcc-switches is similar to -frecord-gcc-switches, but saves the internal GCC switches into DWARF. Lastly, -fverbose-asm option saves the switches into the assembly file but that information never makes it to the object files. -grecord-gcc-switches also allows to match the options used to the actual generated code while both -frecord-gcc-switches and --record-gcc-command-line end up as ELF comment sections not associated with particular code pieces. So IMHO anything but -grecord-gcc-switches is quite useless in case options used do not match for all object files. Richard. I hear that. I am definitely not arguing against the use cases where -grecord-gcc-switches is the best option. But this patch is coming from the point of view of a different use case at hand. We need to have the frontend command line saved in a way that it can be later extracted from the ELF object (including shared libraries and binaries) and be used to re-compile individual objects, producing more or less the same generated code. This patch is not enough to get the exact same output, but it is necessary for us. Correct me if I am wrong, but none of the existing options are very helpful when it comes to asking gcc to do the same thing that it did while creating that object. They hint some things about the command line but do not help much reconstructing the command line. That is what I mean when I say this option is not about the internals of gcc. It is definitely not about knowing what's passed to the backend. As long as this option's output is concerned, gcc is a blackbox. This is about the command-line, about higher-level users' interaction with the driver and nothing more. Best regards Egeyar We're now using git-style commit messages with self-contained explanation / justification of the change being committed. This means that one of the commit messages (not just message 0, whose contents don't go in a commit message) for an individual patch should have the explanation, which should include the self-contained justification by reference to comparison with other existing similar options. People should be able to find the relevant information in the commit without needing to search the list archives for reviews of a previous patch version. Thanks for telling me. I will extend the above comparison according to the questions I might receive. Then I'll add it, together with the explanation in the cover letter, into the commit message of the second patch. Regards Egeyar
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
I'm sending the updated patch based on Egeyar's work. It utilizes a new environmental variable and uses the currently existing -frecord-gcc-switches option. Thoughts? I am leaving it to the more experienced to comment on redefining the functionality of -frecord-gcc-switches. The code seems pretty neat to me. Thanks Martin! Best regards Egeyar
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 6:33 PM, Jakub Jelinek wrote: On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote: On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Can we please wait? I'm really convinced we do not want one another very similar functionality. I would definitely recommend to change the semantics of -frecord-gcc-switches to what the patchset does. That's why I added Nick as he's the author of the original implementation. Reasoning is provided in my previous email. Also, what is the reason for storing the option in a file, rather than say putting them into an environment variable from which the backend can pick it up? I needed to pass that information from one process to the other and I picked a way to do so. Moreover, passing files is what gcc does; therefore, it didn't seem unnatural to do so. Furthermore, gcc specs already has its way to create temporary file names so I didn't have to worry about what level of thread safety is acceptable. If you tell me why an environment variable is better, we can discuss and I might implement that too. I must say I don't really see advantages of this over -grecord-gcc-switches, recording all options looks very bloaty and will include mostly stuff you don't really care about (such as, e.g. the -I options without knowing what was the current directory when the source file has been compiled), on the other side will not record interesting options that -grecord-gcc-switches records (say, if some code is compiled with -march=native, this new option will record that, rather than what it really is), 1) -grecord-gcc-switches is quite similar to -frecord-gcc-switches and my first and main arguement about the latter is valid for the former: You cannot easily construct the right command line to invoke the same compilation from the switches. Switches are useful to check that the compiler is doing the expected. The command line is useful when you want the compiler to do the same thing again. 2) The output is exactly what gcc takes as command line to make that compilation. It is bloaty, surely, when so is what's provided to gcc to make it compile as desired. On the other hand, -grecord-gcc-switches is just much bloatier since it does not work alone. It's embedded in dwarf. 3) Many systems automatically strip dwarf, together with this information. I do not think there is an easy way to separate this information and keep it. Since this comes up often, I'd like to emphasize that the internal switches of gcc are different than the user-given options of gcc. The purpose of this new option is really to catch the user given options of gcc and only them, so much so that I mark it SWITCH_IGNORE within the specs to avoid it propagated to any child processes of gcc. This option is simply to retrieve how the call was made to GCC. Currently, there are no other options giving us this information. but I won't stand in a way unless such an option would be on by default. I totally agree that this should not be on by default. Best regards Egeyar
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 6:23 PM, Martin Liška wrote: On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Can we please wait? I'm really convinced we do not want one another very similar functionality. I am sorry. I thought Richard's comment was only about the third patch, which is also necessary for the current -frecord-gcc-switches. Therefore, my reply was only for that patch as well. Regards Egeyar I would definitely recommend to change the semantics of -frecord-gcc-switches to what the patchset does. That's why I added Nick as he's the author of the original implementation. Reasoning is provided in my previous email. Martin
Re: [PATCH v2 0/3] Introduce a new GCC option, --record-gcc-command-line
On 3/4/20 5:28 PM, Egeyar Bagcioglu wrote: On 3/4/20 1:18 AM, Fangrui Song wrote: On 2020-03-03, Joseph Myers wrote: On Tue, 3 Mar 2020, Egeyar Bagcioglu wrote: Although we discussed after the submission of the first version that there are several other options performing similar tasks, I believe we established that there is still a need for this specific functionality. Therefore, I am skipping in this email the comparison between this option and the existing options with similarities. Mentioning -frecord-gcc-switches will be much appreciated. How is the new .GCC.command.line different? Does it still have the SHF_MERGE | SHF_STRINGS flag? If you change the flags, the .GCC.command.line section may not play with another object file (generated by -frecord-gcc-switches) whose .GCC.command.line is SHF_MERGE | SHF_STRINGS. When both -frecord-gcc-switches and --record-command-line are specified, is it an error? This option is similar to -frecord-gcc-switches. However, they have three fundamental differences: Firstly, -frecord-gcc-switches saves the internal state after the argv is processed and passed by the driver. As opposed to that, --record-gcc-command-line saves the command-line as received by the driver, with the exception of extending @files first. Secondly, -frecord-gcc-switches saves the switches as separate entries into a mergeable string section. Therefore, the entries belonging to different object files get mixed up after being linked. The new --record-gcc-command-line, on the other hand, creates one entry per invocation. By doing so, it makes it clear which options were used together in a single gcc invocation. Lastly, --record-gcc-command-line also adds the version of the gcc into this single entry to make it clear which version of gcc was called with any given command line. This is useful in cases where .comment section reports multiple versions. I should add here that both Martin Liska and Nick Clifton mentioned during the last round of discussion the importance of capturing options such as -D_FORTIFY_SOURCE which is apparently not distinguished by --frecord-gcc-switches. The option I am suggesting, --record-gcc-command-line, saves that without any special case handling. While there are also similarities between the implementations of these two options, those implementations are completely independent. These commands can be used separately or together without issues. I used the same section that -frecord-gcc-switches uses on purpose, so that they can also be used together to save both the command line given to GCC and the internal switches passed by GCC. Here is an old example from the previous discussion, calling g++ with both -frecord-gcc-switches and --record-gcc-command-line for comparison: [egeyar@localhost save-commandline]$ g++ main.c --record-gcc-command-line -frecord-gcc-switches [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.0 20191025 (experimental) : g++ main.c --record-gcc-command-line -frecord-gcc-switches [ 5c] -D_GNU_SOURCE [ 6a] main.c [ 71] -mtune=generic [ 80] -march=x86-64 [ 8e] --record-gcc-command-line /tmp/ccgC4ZtS.cmdline Above, the first entry in the .GCC.command.line section is saved by --record-gcc-command-line; while the rest of the entries are added by -frecord-gcc-switches. Regards Egeyar The option -grecord-gcc-switches is similar to -frecord-gcc-switches, but saves the internal GCC switches into DWARF. Lastly, -fverbose-asm option saves the switches into the assembly file but that information never makes it to the object files. We're now using git-style commit messages with self-contained explanation / justification of the change being committed. This means that one of the commit messages (not just message 0, whose contents don't go in a commit message) for an individual patch should have the explanation, which should include the self-contained justification by reference to comparison with other existing similar options. People should be able to find the relevant information in the commit without needing to search the list archives for reviews of a previous patch version. Thanks for telling me. I will extend the above comparison according to the questions I might receive. Then I'll add it, together with the explanation in the cover letter, into the commit message of the second patch. Regards Egeyar
Re: [PATCH v2 0/3] Introduce a new GCC option, --record-gcc-command-line
On 3/4/20 1:18 AM, Fangrui Song wrote: On 2020-03-03, Joseph Myers wrote: On Tue, 3 Mar 2020, Egeyar Bagcioglu wrote: Although we discussed after the submission of the first version that there are several other options performing similar tasks, I believe we established that there is still a need for this specific functionality. Therefore, I am skipping in this email the comparison between this option and the existing options with similarities. Mentioning -frecord-gcc-switches will be much appreciated. How is the new .GCC.command.line different? Does it still have the SHF_MERGE | SHF_STRINGS flag? If you change the flags, the .GCC.command.line section may not play with another object file (generated by -frecord-gcc-switches) whose .GCC.command.line is SHF_MERGE | SHF_STRINGS. When both -frecord-gcc-switches and --record-command-line are specified, is it an error? This option is similar to -frecord-gcc-switches. However, they have three fundamental differences: Firstly, -frecord-gcc-switches saves the internal state after the argv is processed and passed by the driver. As opposed to that, --record-gcc-command-line saves the command-line as received by the driver, with the exception of extending @files first. Secondly, -frecord-gcc-switches saves the switches as separate entries into a mergeable string section. Therefore, the entries belonging to different object files get mixed up after being linked. The new --record-gcc-command-line, on the other hand, creates one entry per invocation. By doing so, it makes it clear which options were used together in a single gcc invocation. Lastly, --record-gcc-command-line also adds the version of the gcc into this single entry to make it clear which version of gcc was called with any given command line. This is useful in cases where .comment section reports multiple versions. While there are also similarities between the implementations of these two options, those implementations are completely independent. These commands can be used separately or together without issues. I used the same section that -frecord-gcc-switches uses on purpose, so that they can also be used together to save both the command line given to GCC and the internal switches passed by GCC. The option -grecord-gcc-switches is similar to -frecord-gcc-switches, but saves the internal GCC switches into DWARF. Lastly, -fverbose-asm option saves the switches into the assembly file but that information never makes it to the object files. We're now using git-style commit messages with self-contained explanation / justification of the change being committed. This means that one of the commit messages (not just message 0, whose contents don't go in a commit message) for an individual patch should have the explanation, which should include the self-contained justification by reference to comparison with other existing similar options. People should be able to find the relevant information in the commit without needing to search the list archives for reviews of a previous patch version. Thanks for telling me. I will extend the above comparison according to the questions I might receive. Then I'll add it, together with the explanation in the cover letter, into the commit message of the second patch. Regards Egeyar
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 4:34 PM, Andreas Schwab wrote: On Mär 04 2020, Richard Biener wrote: --record-gcc-command-line is not a FSF GCC option, there's -frecord-gcc-switches though which --record-gcc-command-line is translated to -frecord-gcc-switches by the driver. That happens for all double-dash options that match an -f option. I think there is a misunderstanding here. I have just implemented --record-gcc-command-line. I have been testing it and no it does not act as -frecord-gcc-switches. Regards Egeyar Andreas.
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 10:00 AM, Richard Biener wrote: On Tue, Mar 3, 2020 at 5:41 PM Egeyar Bagcioglu wrote: This patch is for .GCC.command.line sections in LTO objects to be copied into the final objects as in the following example: [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g --record-gcc-command-line [56] 10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 --record-gcc-command-line is not a FSF GCC option, there's -frecord-gcc-switches though which (also) populates .GCC.command.line Right. This is also necessary for preserving the outcome of -frecord-gcc-switches option and that issue is reported to me by Martin Liska. OK. Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Regards Egeyar Thanks, Richard. Regards Egeyar libiberty: 2020-02-27 Egeyar Bagcioglu * simple-object.c (handle_lto_debug_sections): Name ".GCC.command.line" among debug sections to be copied over from lto objects. --- libiberty/simple-object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c index d9c648a..3b3ca9c 100644 --- a/libiberty/simple-object.c +++ b/libiberty/simple-object.c @@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename) COMDAT sections in objects produced by GCC. */ else if (strcmp (name, ".comment") == 0) return strcpy (newname, name); + /* Copy over .GCC.command.line section under the same name if present. */ + else if (strcmp (name, ".GCC.command.line") == 0) +return strcpy (newname, name); free (newname); return NULL; } -- 1.8.3.1
Re: [PATCH v2 0/3] Introduce a new GCC option, --record-gcc-command-line
On 3/3/20 3:44 PM, Egeyar Bagcioglu wrote: In addition to the new test case, I built binutils as my test case after passing this option to CFLAGS. The added .GCC.command.line section of ld.bfd listed many compile commands as expected. Tested on x86_64-pc-linux-gnu. As mentioned above, I used binutils as the sample project to compile with --record-gcc-command-line. I am attaching to this email the file produced by the command "readelf -p .GCC.command.line ld/ld-new > ld.commandline.out". You can see in it, all the gcc invocations whose output is merged into ld.bfd. Regards Egeyar String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-catools4/ld/../include -I../../binutils-catools4/ld/../zlib --record-gcc-command-line -DENABLE_PLUGINS -DLOCALEDIR="/home/egeyar/catools4repos/binutils-catools4-build/share/locale" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=TRUE -DELF_SHLIB_LIST_OPTIONS=TRUE -DELF_PLT_UNWIND_LIST_OPTIONS=TRUE --record-gcc-command-line -MT ldgram.o -MD -MP -MF .deps/ldgram.Tpo -c -o ldgram.o ldgram.c -Wno-error [ 282] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-catools4/ld/../include -I../../binutils-catools4/ld/../zlib --record-gcc-command-line -DENABLE_PLUGINS -DLOCALEDIR="/home/egeyar/catools4repos/binutils-catools4-build/share/locale" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=TRUE -DELF_SHLIB_LIST_OPTIONS=TRUE -DELF_PLT_UNWIND_LIST_OPTIONS=TRUE --record-gcc-command-line -MT ldlex-wrapper.o -MD -MP -MF .deps/ldlex-wrapper.Tpo -c -o ldlex-wrapper.o ../../binutils-catools4/ld/ldlex-wrapper.c -Wno-error [ 53b] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-catools4/ld/../include -I../../binutils-catools4/ld/../zlib --record-gcc-command-line -DENABLE_PLUGINS -DLOCALEDIR="/home/egeyar/catools4repos/binutils-catools4-build/share/locale" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=TRUE -DELF_SHLIB_LIST_OPTIONS=TRUE -DELF_PLT_UNWIND_LIST_OPTIONS=TRUE --record-gcc-command-line -MT lexsup.o -MD -MP -MF .deps/lexsup.Tpo -c -o lexsup.o ../../binutils-catools4/ld/lexsup.c [ 7cd] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-catools4/ld/../include -I../../binutils-catools4/ld/../zlib --record-gcc-command-line -DENABLE_PLUGINS -DLOCALEDIR="/home/egeyar/catools4repos/binutils-catools4-build/share/locale" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=TRUE -DELF_SHLIB_LIST_OPTIONS=TRUE -DELF_PLT_UNWIND_LIST_OPTIONS=TRUE --record-gcc-command-line -MT ldlang.o -MD -MP -MF .deps/ldlang.Tpo -c -o ldlang.o ../../binutils-catools4/ld/ldlang.c [ a5f] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-catools4/ld/../include -I../../binutils-catools4/ld/../zlib --record-gcc-command-line -DENABLE_PLUGINS -DLOCALEDIR="/home/egeyar/catools4repos/binutils-catools4-build/share/locale" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=TRUE -DELF_SHLIB_LIST_OPTIONS=TRUE -DELF_PLT_UNWIND_LIST_OPTIONS=TRUE --record-gcc-command-line -MT mri.o -MD -MP -MF .deps/mri.Tpo -c -o mri.o ../../binutils-catools4/ld/mri.c [ ce5] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-catools4/ld/../include -I../../binutils-catools4/ld/../zlib --record-gcc-command-line -DENABLE_PLUGINS -DLOCALEDIR="/home/egeyar/catools4repos/binutils-catools4-build/share/locale" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=TRUE -DELF_SHLIB_LIST_OPTIONS=TRUE -DELF_PLT_UNWIND_LIST_OPTIONS=TRUE --record-gcc-command-line -MT ldctor.o -MD -MP -MF .deps/ldctor.Tpo -c -o ldctor.o ../../binutils-catools4/ld/ldctor.c [ f77] 10.0.1 20200227 (experimental) : gcc -DHAVE_CONFIG_H -I. -I../../binutils-catools4/ld -I. -I../../binutils-catools4/ld -I../bfd -I../../binutils-catools4/ld/../bfd -I../../binutils-
[PATCH v2 1/3] Introduce dg-require-target-object-format
gcc/testsuite/:
2020-02-27 Egeyar Bagcioglu
* lib/target-supports-dg.exp (dg-require-target-object-format): New.
---
gcc/testsuite/lib/target-supports-dg.exp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/gcc/testsuite/lib/target-supports-dg.exp
b/gcc/testsuite/lib/target-supports-dg.exp
index 2a21424..9678a66 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -164,6 +164,17 @@ proc dg-require-dll { args } {
set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
}
+# If this target does not produce the given object format skip this test.
+
+proc dg-require-target-object-format { args } {
+if [string equal [gcc_target_object_format] [lindex $args 1] ] {
+ return
+}
+
+upvar dg-do-what dg-do-what
+set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+}
+
# If this host does not support an ASCII locale, skip this test.
proc dg-require-ascii-locale { args } {
--
1.8.3.1
[PATCH v2 2/3] Introduce the gcc option --record-gcc-command-line
gcc:
2020-02-27 Egeyar Bagcioglu
* common.opt (--record-gcc-command-line): New option.
* config/elfos.h (TARGET_ASM_RECORD_GCC_COMMAND_LINE): Define as
elf_record_gcc_command_line.
* doc/tm.texi: Regenerate.
* doc/tm.texi.in (TARGET_ASM_RECORD_GCC_COMMAND_LINE): Introduce.
(TARGET_ASM_RECORD_GCC_COMMAND_LINE_SECTION): Likewise.
* gcc.c (_gcc_argc): New static variable.
(_gcc_argv): Likewise.
(record_gcc_command_line_spec_function): New function.
(cc1_options): Handle --record-gcc-command-line.
(static_spec_functions): Add record_gcc_command_line_spec_function
with pseudo name record-gcc-command-line.
(driver::main): Call set_commandline.
(driver::set_commandline): Declare.
* gcc.h (driver::set_commandline): Declare.
* target.def (record_gcc_command_line): A new hook.
(record_gcc_command_line_section): A new hookpod.
* target.h (elf_record_gcc_command_line): Declare.
* toplev.c (init_asm_output): Check for gcc_command_line_file and
call record_gcc_command_line if necessary.
* varasm.c: Include "version.h".
(elf_record_gcc_command_line): Define.
gcc/testsuite:
2020-02-27 Egeyar Bagcioglu
* c-c++-common/record-gcc-command-line.c: New.
---
gcc/common.opt | 4 +++
gcc/config/elfos.h | 5 +++
gcc/doc/tm.texi| 22
gcc/doc/tm.texi.in | 4 +++
gcc/gcc.c | 41 ++
gcc/gcc.h | 1 +
gcc/target.def | 30
gcc/target.h | 3 ++
.../c-c++-common/record-gcc-command-line.c | 8 +
gcc/toplev.c | 13 +++
gcc/varasm.c | 40 +
11 files changed, 171 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/record-gcc-command-line.c
diff --git a/gcc/common.opt b/gcc/common.opt
index fa9da50..1bacded 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -391,6 +391,10 @@ Driver Alias(print-sysroot-headers-suffix)
-profile
Common Alias(p)
+-record-gcc-command-line
+Common NoDriverArg Separate Var(gcc_command_line_file)
+Record the command line making this gcc call in the produced object file.
+
-save-temps
Driver Alias(save-temps)
diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
index 74a3eaf..1d5f447 100644
--- a/gcc/config/elfos.h
+++ b/gcc/config/elfos.h
@@ -462,6 +462,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
#undef TARGET_ASM_RECORD_GCC_SWITCHES
#define TARGET_ASM_RECORD_GCC_SWITCHES elf_record_gcc_switches
+/* Allow the use of the --record-gcc-command-line switch via the
+ elf_record_gcc_command_line function defined in varasm.c. */
+#undef TARGET_ASM_RECORD_GCC_COMMAND_LINE
+#define TARGET_ASM_RECORD_GCC_COMMAND_LINE elf_record_gcc_command_line
+
/* A C statement (sans semicolon) to output to the stdio stream STREAM
any text necessary for declaring the name of an external symbol
named NAME which is referenced in this compilation but not defined.
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 19985ad..0a8ef03 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -8113,6 +8113,28 @@ ELF implementation of the
@code{TARGET_ASM_RECORD_GCC_SWITCHES} target
hook.
@end deftypevr
+@deftypefn {Target Hook} int TARGET_ASM_RECORD_GCC_COMMAND_LINE ()
+Provides the target with the ability to record the command line that
+has been passed to the compiler driver. The @var{gcc_command_line_file}
+variable specifies the intermediate file that holds the command line.
+
+The return value must be zero. Other return values may be supported
+in the future.
+
+By default this hook is set to NULL, but an example implementation,
+@var{elf_record_gcc_command_line}, is provided for ELF based targets.
+it records the command line as ASCII text inside a new, mergable string
+section in the assembler output file. The name of the new section is
+provided by the @code{TARGET_ASM_RECORD_GCC_COMMAND_LINE_SECTION}
+target hook.
+@end deftypefn
+
+@deftypevr {Target Hook} {const char *}
TARGET_ASM_RECORD_GCC_COMMAND_LINE_SECTION
+This is the name of the section that will be created by the example
+ELF implementation of the @code{TARGET_ASM_RECORD_GCC_COMMAND_LINE}
+target hook.
+@end deftypevr
+
@need 2000
@node Data Output
@subsection Output of Data
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 1a16150..174840b 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5192,6 +5192,10 @@ It must not be modified by command-line option
processing.
@hook TARGET_ASM_RECORD_GCC_SWITCHES_SECTIO
[PATCH v2 0/3] Introduce a new GCC option, --record-gcc-command-line
Hello, I would like to propose the second version of the patches which introduce a compile option --record-gcc-command-line. When passed to gcc, it saves the command line invoking gcc into the produced object file. The option makes it trivial to trace back with which command a file was compiled and by which version of the gcc. It helps with debugging, reproducing bugs and repeating the build process. The reviews addressed in this version include indentation changes, error handling and corner case coverage pointed out by Segher Boessenkool in the first two patches; while the new third patch is another corner case (lto) coverage requested by Martin Liska. Although we discussed after the submission of the first version that there are several other options performing similar tasks, I believe we established that there is still a need for this specific functionality. Therefore, I am skipping in this email the comparison between this option and the existing options with similarities. This functionality operates as the following: It saves gcc's argv into a temporary file, and passes --record-gcc-command-line to cc1 or cc1plus. The functionality of the backend is implemented via a hook. This patch includes an example implementation of the hook for elf targets: elf_record_gcc_command_line function. This function reads the given file and writes gcc's version and the command line into a mergeable string section, .GCC.command.line. It creates one entry per invocation. By doing so, it makes it clear which options were used together in a single gcc invocation, even after linking. Here is an *example usage* of the option: [egeyar@localhost save-commandline]$ gcc main.c --record-gcc-command-line [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : gcc main.c --record-gcc-command-line The following is a *second example* calling g++ with -save-temps and a repetition of options, where --save-temps saves the intermediate file, main.cmdline in this case. You can see that the options are recorded unprocessed: [egeyar@localhost save-commandline]$ g++ main.c -save-temps --record-gcc-command-line -O0 -O2 -O3 -DFORTIFY=2 --record-gcc-command-line [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : g++ main.c -save-temps --record-gcc-command-line -O0 -O2 -O3 -DFORTIFY=2 --record-gcc-command-line The first patch of this three-patch-series only extends the testsuite machinery, while the second patch implements the functionality and adds a test case for it. The third patch that alters libiberty is to make sure the .GCC.command.line section in LTO objects survive the linking and appear in the linked object. In addition to the new test case, I built binutils as my test case after passing this option to CFLAGS. The added .GCC.command.line section of ld.bfd listed many compile commands as expected. Tested on x86_64-pc-linux-gnu. Please review the patches, let me know what you think and apply if appropriate. Regards Egeyar Egeyar Bagcioglu (3): Introduce dg-require-target-object-format Introduce the gcc option --record-gcc-command-line Keep .GCC.command.line sections of LTO objetcs. gcc/common.opt | 4 +++ gcc/config/elfos.h | 5 +++ gcc/doc/tm.texi| 22 gcc/doc/tm.texi.in | 4 +++ gcc/gcc.c | 41 ++ gcc/gcc.h | 1 + gcc/target.def | 30 gcc/target.h | 3 ++ .../c-c++-common/record-gcc-command-line.c | 8 + gcc/testsuite/lib/target-supports-dg.exp | 11 ++ gcc/toplev.c | 13 +++ gcc/varasm.c | 40 + libiberty/simple-object.c | 3 ++ 13 files changed, 185 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/record-gcc-command-line.c -- 1.8.3.1
[PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
This patch is for .GCC.command.line sections in LTO objects to be copied into the final objects as in the following example: [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g --record-gcc-command-line [56] 10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 Regards Egeyar libiberty: 2020-02-27 Egeyar Bagcioglu * simple-object.c (handle_lto_debug_sections): Name ".GCC.command.line" among debug sections to be copied over from lto objects. --- libiberty/simple-object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c index d9c648a..3b3ca9c 100644 --- a/libiberty/simple-object.c +++ b/libiberty/simple-object.c @@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename) COMDAT sections in objects produced by GCC. */ else if (strcmp (name, ".comment") == 0) return strcpy (newname, name); + /* Copy over .GCC.command.line section under the same name if present. */ + else if (strcmp (name, ".GCC.command.line") == 0) +return strcpy (newname, name); free (newname); return NULL; } -- 1.8.3.1
Re: [PATCH 1/2] Introduce dg-require-target-object-format
On 11/14/19 3:51 AM, Hans-Peter Nilsson wrote:
On Thu, 7 Nov 2019, Egeyar Bagcioglu wrote:
On 11/7/19 8:47 AM, Segher Boessenkool wrote:
On Wed, Nov 06, 2019 at 06:21:33PM +0100, Egeyar Bagcioglu wrote:
+proc dg-require-target-object-format { args } {
+if { [gcc_target_object_format] == [lindex $args 1] } {
+ return
+}
"==" for strings is dangerous. Use "eq" or "string equal"?
I see many "string match"es. I will make the change.
Please instead use "string equal".
Code in target-supports-dg.exp is not a trustworthy reference.
I suggest RTFM <http://www.tcl.tk/>, where you can see that
"string match" does a glob compare.
brgds, H-P
I added the change to my local copy. Thanks for the correction and for
the reasoning.
Regards
Egeyar
Re: [PATCH 0/2] Introduce a new GCC option, --record-gcc-command-line
On 11/13/19 10:37 AM, Martin Liška wrote:
On 11/7/19 3:50 PM, Egeyar Bagcioglu wrote:
On 11/7/19 10:24 AM, Martin Liška wrote:
On 11/6/19 6:21 PM, Egeyar Bagcioglu wrote:
Hello,
Hello.
Thanks for your detailed reply Martin. You'll find my reply inline.
Since you added Nick Clifton to your following reply, I am adding him
to this email too. He is not only the author of annobin, he also
submitted the -frecord-gcc-switches to GCC. I agree that this
discussion can benefit from his input.
I would like to propose the following patches which introduce a
compile option --record-gcc-command-line. When passed to gcc, it
saves the command line option into the produced object file. The
option makes it trivial to trace back how a file was compiled and
by which version of the gcc. It helps with debugging, reproducing
bugs and repeating the build process.
I like your motivation, we as SUSE would like to have a similar
functionality. But the current approach has some limitations that
make it not usable (will explain later).
I am glad you agree with the motivation. Let me answer below the
other concerns that you have.
This option is similar to -frecord-gcc-switches. However, they have
three fundamental differences: Firstly, -frecord-gcc-switches saves
the internal state after the argv is processed and passed by the
driver. As opposed to that, --record-gcc-command-line saves the
I would not name it as a fundamental changes, it's doing very
similar to what -frecord-gcc-switches does.
It is very similar; however, I still insist that what I outlined are
fundamental differences. As I mentioned in my previous email, I built
binutils as my test-case-project. I attach to this email the output
of "readelf -p .GCC.command.line ld/ld-new", so that you can see how
well the output is merged in general. Please take a look. It saves
the command line *as is* and as one entry per invocation.
Hello.
Ok, works for me and I'm glad you also wrote a counterpart for
bintuils which can easily present the information to a user.
I am glad you liked the output. It is output by readelf without any
additional patches.
For the record, this is just to test and showcase the functionality.
This patch in fact has nothing to do with binutils.
Moreover, we also have one another option -grecord-gcc-switches
that saves command line into DWARF.
As Nick also mentioned many times, -grecord-gcc-switches is in DWARF
and this causes a great disadvantage: it gets stripped out.
Well, that's still something I disagree. I bet RedHat is similarly to
openSUSE also building all packages with a debug info, which
is later stripped and put into a foo-devel package. That's why one can
easily read the compile options from these sub-packages.
My motivation is to write a rpm linter check that will verify that all
object files really used flags that we expect.
I understand your use case. However, some of the use cases we have for
this patch are not for the distros but for the development. Having the
compile options in the object files allows developers to pass around
objects that are compiled differently without needing to tag them
separately. This eases for example the performance analysis. A similar
argument can also be made for reporting bugs.
I believe the -grecord-gcc-switches is moot for the sake of this
discussion. Because I think the discussion surrounding the submission
of -frecord-gcc-switches makes it clear that the necessity to keep
the compile options in the object file is something that is already
agreed on.
Plus there's a Red Hat plugin called Annobin:
https://www.youtube.com/watch?v=uzffr1M-w5M
https://developers.redhat.com/blog/2018/02/20/annobin-storing-information-binaries/
I am aware of annobin, which is already released as a part of RHEL8.
I think it is much superior to what I am aiming here. The sole
purpose of this patch is to keep the command line options in the
object file. I believe this is a basic functionality that should be
supported by the GCC itself, without requiring a plugin.
I fully aggree with you.
In other words, I think pushing a different build of a GCC plugin for
each GCC version we use on each distro (i.e. versions-times-distros
many plugin builds) is an overkill for such a basic need.
Yep.
Those who already use annobin for any of its many use cases, might of
course prefer it over this functionality. For the rest of the distros
and the GCC versions, I believe this patch is quite useful and
extendable for its quite basic purpose.
Main limitation of current approach (and probably the suggested
patch) are:
a) it does not print per function options, which can be modified
with __attribute__ (or pragma):
$ cat demo.c
int foo()
{
return 1;
}
#pragma GCC optimize ("-O3")
int bar()
{
return 0;
}
int main()
{
return 0;
}
I understand the need here. However, the purpose of this patch is
only to s
Re: [PATCH 2/2] Introduce the gcc option --record-gcc-command-line
On 11/7/19 7:57 PM, Segher Boessenkool wrote: Hi! On Thu, Nov 07, 2019 at 06:44:17PM +0100, Egeyar Bagcioglu wrote: On 11/7/19 9:03 AM, Segher Boessenkool wrote: + ASM_OUTPUT_ASCII(asm_out_file, cmdline, cmdline_length); +} + cmdline[0] = 0; + ASM_OUTPUT_ASCII(asm_out_file, cmdline, 1); + + /* The return value is currently ignored by the caller, but must be 0. */ + return 0; +} A temporary file like this isn't so great. GCC operates with temporary files, doesn't it? What is the concern that is specific to this one? That is the most reasonable way I found to pass the argv of gcc to child processes for saving. Other ways of passing it that I could think of, or the idea of saving it in the driver were actually very bad ideas. Oh, this is for passing something to another process? I guess I didn't read it closely enough, sorry, never mind. Opening a file as "r" but then accessing it with "fread" is peculiar, too. I am not sure what you mean here. Is it that you prefer "wb" and "rb" instead of "w" and "r"? I thought it was enough to use a consistent pair. I'd use fgets or similar, not fread. Two things made me prefer fread over fgets here: 1) Although I am reading a string, I do not need each read character to be checked against newline. I just need to read till end-of-file. 2) fread returns the number of elements read which I later use. If I used fgets, I'd need to call strlen or so afterwards to get the string size. Let me know please if you disagree or if there are advantages / disadvantages that I omit. Regards Egeyar
Re: [PATCH 2/2] Introduce the gcc option --record-gcc-command-line
Hello again Segher!
On 11/7/19 9:03 AM, Segher Boessenkool wrote:
Hi!
On Wed, Nov 06, 2019 at 06:21:34PM +0100, Egeyar Bagcioglu wrote:
+static const char *
+record_gcc_command_line_spec_function(int argc ATTRIBUTE_UNUSED, const char
**argv)
+{
+ const char *filename = argv[0];
+ FILE *out = fopen (filename, "w");
+ if (out)
+{
+ fputs (_gcc_argv[0], out);
+ for (int i = 1; i < _gcc_argc; i += 1)
+ {
+ fputc (' ', out);
+ fputs (_gcc_argv[i], out);
+ }
+ fclose (out);
+}
+ return filename;
+}
Pet peeve: just use fprintf?
okay.
If there is an error, should this return 0 or indicate error some way?
Making the error silent ("if (out)") seems weird, otherwise -- if it is
on purpose, a comment might help.
It was on purpose so that this does not interfere with the builds.
However, re-watching today Nick's talk at Cauldron where he mentions
it's good to fail even in such cases, I am rethinking if we would like
to error out here. If anyone has any preference on this, I'd like to hear.
+int
+elf_record_gcc_command_line ()
+{
+ char cmdline[256];
+ section * sec;
section *sec;
right.
+ sec = get_section (targetm.asm_out.record_gcc_switches_section,
+SECTION_DEBUG | SECTION_MERGE
+| SECTION_STRINGS | (SECTION_ENTSIZE & 1),
+NULL);
+ switch_to_section (sec);
+
+ ASM_OUTPUT_ASCII(asm_out_file, version_string, strlen(version_string));
+
+ FILE *out_stream = fopen (gcc_command_line_file, "r");
+ if (out_stream)
+{
+ ASM_OUTPUT_ASCII(asm_out_file, " : ", 3);
+ ssize_t cmdline_length;
+ while ((cmdline_length = fread(cmdline, 1, 256, out_stream)))
fread (
(and many more like it).
okay, I will fix them. Thanks for catching.
+ ASM_OUTPUT_ASCII(asm_out_file, cmdline, cmdline_length);
+}
+ cmdline[0] = 0;
+ ASM_OUTPUT_ASCII(asm_out_file, cmdline, 1);
+
+ /* The return value is currently ignored by the caller, but must be 0. */
+ return 0;
+}
A temporary file like this isn't so great.
GCC operates with temporary files, doesn't it? What is the concern that
is specific to this one? That is the most reasonable way I found to pass
the argv of gcc to child processes for saving. Other ways of passing it
that I could think of, or the idea of saving it in the driver were
actually very bad ideas.
Opening a file as "r" but then
accessing it with "fread" is peculiar, too.
I am not sure what you mean here. Is it that you prefer "wb" and "rb"
instead of "w" and "r"? I thought it was enough to use a consistent pair.
HTH,
It does! Thanks a lot for the review!
Regards
Egeyar
Re: [PATCH 1/2] Introduce dg-require-target-object-format
On 11/7/19 6:17 PM, jose.march...@oracle.com wrote:
On 11/7/19 8:47 AM, Segher Boessenkool wrote:
> Hi!
>
> On Wed, Nov 06, 2019 at 06:21:33PM +0100, Egeyar Bagcioglu wrote:
>> gcc/testsuite/ChangeLog:
>> 2019-11-06 Egeyar Bagcioglu
>>
>> * lib/target-supports-dg.exp: Define
dg-require-target-object-format.
> * lib/target-supports-dg.exp (dg-require-target-object-format): New.
Right, thanks for the correction!
>> +proc dg-require-target-object-format { args } {
>> +if { [gcc_target_object_format] == [lindex $args 1] } {
>> + return
>> +}
> "==" for strings is dangerous. Use "eq" or "string equal"?
I see many "string match"es. I will make the change.
Just as a note, though: Why is it dangerous? In C, for example, this
would be a pointer comparison and consistently fail. In many other
languages, it is indeed a string comparison and works well.
I am asking also because I see "==" for variable vs literal strings in
gcc/testsuite/lib. As opposed to C-like languages that consistently
fail them, these seem to work. If you still think this is dangerous,
I'd love to know why. Also, if so, someone might want to check the
library.
Because if the string happens to look like a number Tcl will perform
arithmetic comparison instead of lexicographic comparison, i.e. "01" ==
"1" evaluates to true :)
Oh lovely indeed! I am glad I asked.
Thanks Jose!
Re: [PATCH 1/2] Introduce dg-require-target-object-format
Hi Segher!
On 11/7/19 8:47 AM, Segher Boessenkool wrote:
Hi!
On Wed, Nov 06, 2019 at 06:21:33PM +0100, Egeyar Bagcioglu wrote:
gcc/testsuite/ChangeLog:
2019-11-06 Egeyar Bagcioglu
* lib/target-supports-dg.exp: Define dg-require-target-object-format.
* lib/target-supports-dg.exp (dg-require-target-object-format): New.
Right, thanks for the correction!
+proc dg-require-target-object-format { args } {
+if { [gcc_target_object_format] == [lindex $args 1] } {
+ return
+}
"==" for strings is dangerous. Use "eq" or "string equal"?
I see many "string match"es. I will make the change.
Just as a note, though: Why is it dangerous? In C, for example, this
would be a pointer comparison and consistently fail. In many other
languages, it is indeed a string comparison and works well.
I am asking also because I see "==" for variable vs literal strings in
gcc/testsuite/lib. As opposed to C-like languages that consistently fail
them, these seem to work. If you still think this is dangerous, I'd love
to know why. Also, if so, someone might want to check the library.
Thanks for the review!
Egeyar
Re: [PATCH 0/2] Introduce a new GCC option, --record-gcc-command-line
On 11/7/19 4:13 PM, Nick Clifton wrote: Hi Egeyar, Thanks for including me in this discussion. This option is similar to -frecord-gcc-switches. For the record I will also note that there is -fverbose-asm which does almost the same thing, but only records the options as comments in the assembler. They are never converted into data in the actual object files. Right. It is also worth noting that if your goal is to record how a binary was produced, possibly with an eye to reproducibility, then you may also need to record some environment variables too. That is an important point and in fact, such a need might arise. Even in that case, I would like to keep the options of GCC as modular as possible so that we can pick and drop as the specific use cases require. This one is the one that saves the command line for now. If we implement the aliases you suggested at the end, we can even create aliases that combine them for the user. One thing I found with annobin is that capturing preprocessor options (eg -D_FORTIFY_SOURCE) can be quite hard from inside gcc, since often they have already been processed and discarded. I do not know if this affects your actual patch though. Yes, this was one of Martin's points as well. It is not the case for this patch, though. I have noticed that the current options aim to capture more than the command line, dive into GCC, and therefore miss or discard some options given by the user. This patch only stores *argv* as the driver receives and writes it to the object file blindly. Therefore, capturing options such as -D_FORTIFY_SOURCE is no special case. Really, this patch only answers the simple question of "How did you call GCC?". Speaking of annobin, I will bang the gcc plugin gong again here and say that if your patch is rejected then you might want to consider turning it into a plugin instead. In that way you will not need approval from the gcc maintainers. But of course you will have to maintain and publicise the plugin yourself. Thanks for the suggestion. That will always be in my mind for more ambitious cases. In the case of this specific 160-line patch though, I believe it wouldn't bother us to maintain one more small patch in the GCC packages we distribute. It can be "only at Oracle!". However, for me this is really a basic functionality. Intel's icc has the most similar -sox option too. Thinking back on how many times I said "now, how did we compile this?" in the past, I would like this to be available for all GCC users too, in the spirit of sharing. One other thought occurs to me, which is that if the patch is acceptable, or at least the idea of it, then maybe it would be better to amalgamate all of the current command line recording options into a single version. Eg: --frecord-options=[dwarf,assembler,object] where: --frecord-options=dwarf is a synonym for -grecord-switches --frecord-options=assembler is a synonym for -fverbose-asm --frecord-options=object is a synonym for your option The user could supply one or more of the selectors to have the recording happen in multiple places. Just an idea. This is a very good idea for the user experience! I already pass an argument to cc1; however, we can always simplify the arguments of the driver so that these similar functionalities can be called via one common name plus an option. I really like the idea. Cheers Nick Thanks Nick! Regards Egeyar
Re: [PATCH 0/2] Introduce a new GCC option, --record-gcc-command-line
On 11/7/19 10:24 AM, Martin Liška wrote:
On 11/6/19 6:21 PM, Egeyar Bagcioglu wrote:
Hello,
Hello.
Thanks for your detailed reply Martin. You'll find my reply inline.
Since you added Nick Clifton to your following reply, I am adding him to
this email too. He is not only the author of annobin, he also submitted
the -frecord-gcc-switches to GCC. I agree that this discussion can
benefit from his input.
I would like to propose the following patches which introduce a
compile option --record-gcc-command-line. When passed to gcc, it
saves the command line option into the produced object file. The
option makes it trivial to trace back how a file was compiled and by
which version of the gcc. It helps with debugging, reproducing bugs
and repeating the build process.
I like your motivation, we as SUSE would like to have a similar
functionality. But the current approach has some limitations that make
it not usable (will explain later).
I am glad you agree with the motivation. Let me answer below the other
concerns that you have.
This option is similar to -frecord-gcc-switches. However, they have
three fundamental differences: Firstly, -frecord-gcc-switches saves
the internal state after the argv is processed and passed by the
driver. As opposed to that, --record-gcc-command-line saves the
I would not name it as a fundamental changes, it's doing very similar
to what -frecord-gcc-switches does.
It is very similar; however, I still insist that what I outlined are
fundamental differences. As I mentioned in my previous email, I built
binutils as my test-case-project. I attach to this email the output of
"readelf -p .GCC.command.line ld/ld-new", so that you can see how well
the output is merged in general. Please take a look. It saves the
command line *as is* and as one entry per invocation.
For the record, this is just to test and showcase the functionality.
This patch in fact has nothing to do with binutils.
Moreover, we also have one another option -grecord-gcc-switches
that saves command line into DWARF.
As Nick also mentioned many times, -grecord-gcc-switches is in DWARF and
this causes a great disadvantage: it gets stripped out. I believe the
-grecord-gcc-switches is moot for the sake of this discussion. Because I
think the discussion surrounding the submission of -frecord-gcc-switches
makes it clear that the necessity to keep the compile options in the
object file is something that is already agreed on.
Plus there's a Red Hat plugin called Annobin:
https://www.youtube.com/watch?v=uzffr1M-w5M
https://developers.redhat.com/blog/2018/02/20/annobin-storing-information-binaries/
I am aware of annobin, which is already released as a part of RHEL8. I
think it is much superior to what I am aiming here. The sole purpose of
this patch is to keep the command line options in the object file. I
believe this is a basic functionality that should be supported by the
GCC itself, without requiring a plugin. In other words, I think pushing
a different build of a GCC plugin for each GCC version we use on each
distro (i.e. versions-times-distros many plugin builds) is an overkill
for such a basic need.
Those who already use annobin for any of its many use cases, might of
course prefer it over this functionality. For the rest of the distros
and the GCC versions, I believe this patch is quite useful and
extendable for its quite basic purpose.
Main limitation of current approach (and probably the suggested patch)
are:
a) it does not print per function options, which can be modified with
__attribute__ (or pragma):
$ cat demo.c
int foo()
{
return 1;
}
#pragma GCC optimize ("-O3")
int bar()
{
return 0;
}
int main()
{
return 0;
}
I understand the need here. However, the purpose of this patch is only
to save the command line options. Your example is a change in the source
file. Of course, the source file can change. Even the implementation of
the functions themselves might change. But I believe this is out of the
scope of this patch, which is the command line.
b) we as SUSE are switching to LTO (-flto); doing that each LTO LTRANS
will become one compilation unit and
one will see a misleading command line invocation:
$ gcc -flto -O2 demo2.c -c
$ gcc -flto -O3 demo.c -c
$ gcc demo.o demo2.o -o a.out -frecord-gcc-switches
...
.file ""
.section .GCC.command.line,"MS",@progbits,1
.ascii "-mtune=generic"
.zero 1
.ascii "-march=x86-64"
.zero 1
.ascii "-auxbase-strip a.out.ltrans0.ltrans.o"
.zero 1
.ascii "-O3"
.zero 1
.ascii "-fno-openmp"
.zero 1
.ascii "-fno-openacc"
.zero 1
.ascii "-fno-pie"
.zero 1
.ascii "-frecord-gcc-switches"
.zero 1
.ascii "-fltrans"
.zero 1
.ascii &q
[PATCH 2/2] Introduce the gcc option --record-gcc-command-line
gcc/ChangeLog:
2019-10-21 Egeyar Bagcioglu
* common.opt (--record-gcc-command-line): New option.
* config/elfos.h (TARGET_ASM_RECORD_GCC_COMMAND_LINE): Define
as elf_record_gcc_command_line.
* doc/tm.texi: Regenerate.
* doc/tm.texi.in (TARGET_ASM_RECORD_GCC_COMMAND_LINE): Introduce.
(TARGET_ASM_RECORD_GCC_COMMAND_LINE_SECTION): Likewise.
* gcc.c (_gcc_argc): New static variable.
(_gcc_argv): Likewise.
(record_gcc_command_line_spec_function): New function.
(cc1_options): Add --record-gcc-command-line.
(static_spec_functions): Add record_gcc_command_line_spec_function
with pseudo name record-gcc-command-line.
(driver::main): Call set_commandline.
(driver::set_commandline): Declare.
* gcc.h (driver::set_commandline): Declare.
* target.def (record_gcc_command_line): A new hook.
(record_gcc_command_line_section): A new hookpod.
* target.h (elf_record_gcc_command_line): Declare.
* toplev.c (init_asm_output): Check gcc_command_line_file and
call record_gcc_command_line if necessary.
* varasm.c: Include "version.h".
(elf_record_gcc_command_line): Define.
gcc/testsuite/ChangeLog:
2019-10-30 Egeyar Bagcioglu
* c-c++-common/record-gcc-command-line.c: New test case.
---
gcc/common.opt | 4 +++
gcc/config/elfos.h | 5 +++
gcc/doc/tm.texi| 22
gcc/doc/tm.texi.in | 4 +++
gcc/gcc.c | 41 ++
gcc/gcc.h | 1 +
gcc/target.def | 30
gcc/target.h | 3 ++
.../c-c++-common/record-gcc-command-line.c | 8 +
gcc/toplev.c | 13 +++
gcc/varasm.c | 36 +++
11 files changed, 167 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/record-gcc-command-line.c
diff --git a/gcc/common.opt b/gcc/common.opt
index cc279f4..59d670f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -394,6 +394,10 @@ Driver Alias(print-sysroot-headers-suffix)
-profile
Common Alias(p)
+-record-gcc-command-line
+Common Driver NoDriverArg Separate Var(gcc_command_line_file)
+Record the command line making this gcc call in the produced object file.
+
-save-temps
Driver Alias(save-temps)
diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
index e00d437..5caa9e0 100644
--- a/gcc/config/elfos.h
+++ b/gcc/config/elfos.h
@@ -451,6 +451,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
#undef TARGET_ASM_RECORD_GCC_SWITCHES
#define TARGET_ASM_RECORD_GCC_SWITCHES elf_record_gcc_switches
+/* Allow the use of the --record-gcc-command-line switch via the
+ elf_record_gcc_command_line function defined in varasm.c. */
+#undef TARGET_ASM_RECORD_GCC_COMMAND_LINE
+#define TARGET_ASM_RECORD_GCC_COMMAND_LINE elf_record_gcc_command_line
+
/* A C statement (sans semicolon) to output to the stdio stream STREAM
any text necessary for declaring the name of an external symbol
named NAME which is referenced in this compilation but not defined.
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 915e961..6da5e1b 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -8061,6 +8061,28 @@ ELF implementation of the
@code{TARGET_ASM_RECORD_GCC_SWITCHES} target
hook.
@end deftypevr
+@deftypefn {Target Hook} int TARGET_ASM_RECORD_GCC_COMMAND_LINE ()
+Provides the target with the ability to record the command line that
+has been passed to the compiler driver. The @var{gcc_command_line_file}
+variable specifies the intermediate file that holds the command line.
+
+The return value must be zero. Other return values may be supported
+in the future.
+
+By default this hook is set to NULL, but an example implementation,
+@var{elf_record_gcc_command_line}, is provided for ELF based targets.
+it records the command line as ASCII text inside a new, mergable string
+section in the assembler output file. The name of the new section is
+provided by the @code{TARGET_ASM_RECORD_GCC_COMMAND_LINE_SECTION}
+target hook.
+@end deftypefn
+
+@deftypevr {Target Hook} {const char *}
TARGET_ASM_RECORD_GCC_COMMAND_LINE_SECTION
+This is the name of the section that will be created by the example
+ELF implementation of the @code{TARGET_ASM_RECORD_GCC_COMMAND_LINE}
+target hook.
+@end deftypevr
+
@need 2000
@node Data Output
@subsection Output of Data
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index ac0f049..73ca552 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5192,6 +5192,10 @@ It must not be modified by command-line option
processing.
@hook TARGET_ASM_RECORD_GCC_SWITCH
[PATCH 1/2] Introduce dg-require-target-object-format
gcc/testsuite/ChangeLog:
2019-11-06 Egeyar Bagcioglu
* lib/target-supports-dg.exp: Define dg-require-target-object-format.
---
gcc/testsuite/lib/target-supports-dg.exp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/gcc/testsuite/lib/target-supports-dg.exp
b/gcc/testsuite/lib/target-supports-dg.exp
index e1da57a..e923754 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -164,6 +164,17 @@ proc dg-require-dll { args } {
set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
}
+# If this target does not produce the given object format skip this test.
+
+proc dg-require-target-object-format { args } {
+if { [gcc_target_object_format] == [lindex $args 1] } {
+ return
+}
+
+upvar dg-do-what dg-do-what
+set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+}
+
# If this host does not support an ASCII locale, skip this test.
proc dg-require-ascii-locale { args } {
--
1.8.3.1
[PATCH 0/2] Introduce a new GCC option, --record-gcc-command-line
Hello,
I would like to propose the following patches which introduce a compile option
--record-gcc-command-line. When passed to gcc, it saves the command line option
into the produced object file. The option makes it trivial to trace back how a
file was compiled and by which version of the gcc. It helps with debugging,
reproducing bugs and repeating the build process.
This option is similar to -frecord-gcc-switches. However, they have three
fundamental differences: Firstly, -frecord-gcc-switches saves the internal
state after the argv is processed and passed by the driver. As opposed to that,
--record-gcc-command-line saves the command-line as received by the driver.
Secondly, -frecord-gcc-switches saves the switches as separate entries into a
mergeable string section. Therefore, the entries belonging to different object
files get mixed up after being linked. The new --record-gcc-command-line, on
the other hand, creates one entry per invocation. By doing so, it makes it
clear which options were used together in a single gcc invocation. Lastly,
--record-gcc-command-line also adds the version of the gcc into this single
entry to make it clear which version of gcc was called with any given command
line. This is useful in cases where .comment section reports multiple versions.
While there are also similarities between the implementations of these two
options, they are completely independent. These commands can be used separately
or together without issues. I used the same section that -frecord-gcc-switches
uses on purpose. I could not use the name -frecord-gcc-command-line for this
option; because of a {f*} in the specs, which forwards all options starting
with -f to cc1/cc1plus as is. This is not we want for this option. We would
like to append it a filename as well to pass the argv of the driver to child
processes.
This functionality operates as the following: It saves gcc's argv into a
temporary file, and passes --record-gcc-command-line to cc1 or
cc1plus. The functionality of the backend is implemented via a hook. This patch
includes an example implementation of the hook for elf targets:
elf_record_gcc_command_line function. This function reads the given file and
writes gcc's version and the command line into a mergeable string section,
.GCC.command.line.
Here is an *example usage* of the option:
[egeyar@localhost save-commandline]$ gcc main.c --record-gcc-command-line
[egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out
String dump of section '.GCC.command.line':
[ 0] 10.0.0 20191025 (experimental) : gcc main.c
--record-gcc-command-line
The following is a *second example* calling g++ with -save-temps,
-frecord-gcc-switches, and repetition of options, where --save-temps saves the
intermediate file, main.cmdline in this case. You can see that the options are
recorded unprocessed:
[egeyar@localhost save-commandline]$ g++ main.c -save-temps
--record-gcc-command-line -O0 -O2 -O3 --record-gcc-command-line
[egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out
String dump of section '.GCC.command.line':
[ 0] 10.0.0 20191025 (experimental) : g++ main.c -save-temps
--record-gcc-command-line -O0 -O2 -O3 --record-gcc-command-line
Here is a *third example* calling g++ with both -frecord-gcc-switches and
--record-gcc-command-line for comparison:
[egeyar@localhost save-commandline]$ g++ main.c --record-gcc-command-line
-frecord-gcc-switches
[egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out
String dump of section '.GCC.command.line':
[ 0] 10.0.0 20191025 (experimental) : g++ main.c
--record-gcc-command-line -frecord-gcc-switches
[5c] -D_GNU_SOURCE
[6a] main.c
[71] -mtune=generic
[80] -march=x86-64
[8e] --record-gcc-command-line /tmp/ccgC4ZtS.cmdline
The first patch of this two-patch-series only extends the testsuite machinery,
while the second patch implements this functionality and adds a test case for
it. In addition to that new test case, I built binutils as my test case after
passing this option to CFLAGS. The added .GCC.command.line section of ld listed
many compile commands as expected. Tested on x86_64-pc-linux-gnu.
Please review the patches, let me know what you think and apply if appropriate.
Regards
Egeyar
Egeyar Bagcioglu (2):
Introduce dg-require-target-object-format
Introduce the gcc option --record-gcc-command-line
gcc/common.opt | 4 +++
gcc/config/elfos.h | 5 +++
gcc/doc/tm.texi| 22
gcc/doc/tm.texi.in | 4 +++
gcc/gcc.c | 41 ++
gcc/gcc.h | 1 +
gcc/target.def | 30
gcc/target.h | 3
Re: [PATCH] Fix nrv-1.c false failure on aarch64.
On 10/26/2017 05:03 PM, Jeff Law wrote: On 10/18/2017 10:59 AM, Egeyar Bagcioglu wrote: Hello, Test case "guality.exp=nrv-1.c" fails on aarch64. Optimizations reorder the instructions and cause the value of a variable to be checked before its first assignment. The following patch is moving the break point to the end of the function. Therefore, it ensures that the break point is reached after the assignment instruction is executed. Please review the patch and apply if legitimate. This seems wrong. If I understand the test correctly, we want to break on the line with the assignment to a2.i[4] = 7 and verify that before that line executes that a2.i[0] == 42. Moving the test point to the end of the function seems to defeat the purpose of the test. A breakpoint at the end of the function to test state is pointless as it doesn't reflect what a user is likely to want to do. I'm guessing based on your description that optimization has sunk the assignment to a2.i[0] down past the assignment to a2.i[4]? What optimization did this and what do the dwarf records look like? Jeff Hi Jeff, Thank you for the review. Your analysis of the issue is correct. Indeed, I realized after the first reviews that the test was aiming the debugging experience rather than the functionality of the executable. As a result, I withdraw this patch. Earlier reviewers mentioned Alex Oliva's SFN work could fix this test case as well. I verified that. His patches are currently in review. I am following and waiting for their progress before any further action. Egeyar
Re: [PATCH] Fix nrv-1.c false failure on aarch64.
On 10/20/2017 03:13 PM, Richard Earnshaw (lists) wrote: On 19/10/17 09:14, Richard Biener wrote: I guess Alex work on stmt frontiers will fix this instance? Thank you all for the reviews. I fetched Alex's branches in gcc (aoliva/SFN) and in binutils (users/aoliva/SFN). Using gcc and binutils built from these sources, nrv-1.c test case passes. I am following the progress of his patches. Thanks, Egeyar
[PATCH] Fix nrv-1.c false failure on aarch64.
Hello,
Test case "guality.exp=nrv-1.c" fails on aarch64. Optimizations reorder
the instructions and cause the value of a variable to be checked before
its first assignment. The following patch is moving the
break point to the end of the function. Therefore, it ensures that the
break point is reached after the assignment instruction is executed.
Please review the patch and apply if legitimate.
Egeyar
>From a11fe0b1fcf1867a0fa8c4627e347bda07a4c61b Mon Sep 17 00:00:00 2001
From: Egeyar Bagcioglu <egeyar.bagcio...@oracle.com>
Date: Thu, 12 Oct 2017 06:16:30 -0700
Subject: [PATCH] Fix nrv-1.c false failure on aarch64.
Reordering instructions due to optimization flags is causing nrv-1.c
to fail on aarch64. The value of the variable is checked before the
assignment instruction is reached. This patch is moving the
breakpoint to the end of the function to prevent such false failures.
* gcc.dg/guality/nrv-1.c: Move breakpoint from line 20 to 22.
---
gcc/testsuite/gcc.dg/guality/nrv-1.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/guality/nrv-1.c b/gcc/testsuite/gcc.dg/guality/nrv-1.c
index 2f4e654..77ad462 100644
--- a/gcc/testsuite/gcc.dg/guality/nrv-1.c
+++ b/gcc/testsuite/gcc.dg/guality/nrv-1.c
@@ -17,9 +17,9 @@ f ()
a2.i[0] = 42;
if (a3.i[0] != 0)
abort ();
- a2.i[4] = 7; /* { dg-final { gdb-test 20 "a2.i\[0\]" "42" } } */
+ a2.i[4] = 7;
return a2;
-}
+} /* { dg-final { gdb-test 22 "a2.i\[0\]" "42" } } */
int
main ()
--
1.9.1
