Re: [PATCH 2/2] Introduce the gcc option --record-gcc-command-line
On Fri, Nov 08, 2019 at 12:05:19AM +0100, Egeyar Bagcioglu wrote: > On 11/7/19 7:57 PM, Segher Boessenkool wrote: > >>>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. Somehow I thought fread works differently with text streams than it does. Maybe because you don't often see fread on text streams :-) Sorry for the noise, Segher
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
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. Segher
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 2/2] Introduce the gcc option --record-gcc-command-line
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?
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.
> +int
> +elf_record_gcc_command_line ()
> +{
> + char cmdline[256];
> + section * sec;
section *sec;
> + 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).
> + 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. Opening a file as "r" but then
accessing it with "fread" is peculiar, too.
HTH,
Segher
[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_SWITCHES_SECTION
