[PATCH] D30760: Record command lines in objects built by clang, Clang part

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv closed this revision.
george.burgess.iv added a comment.
Herald added a subscriber: JDevlieghere.

(Committed as noted by echristo; just trying to clean my queue a bit. :) )


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

Committed thusly:
echristo@athyra ~/s/l/t/clang> git svn dcommit
Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
M   lib/Driver/ToolChains/Clang.cpp
M   test/Driver/debug-options.c
Committed r299037


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via cfe-commits]

Not really.

Would you please help me commit it? Also there is another diff for LLVM
part of this bug.

Thanks.

On Wed, Mar 29, 2017 at 1:16 PM, Eric Christopher via Phabricator <
revi...@reviews.llvm.org> wrote:

> echristo accepted this revision.
> echristo added a comment.
> This revision is now accepted and ready to land.
>
> Sounds good. Do you have commit access?
>
>
> https://reviews.llvm.org/D30760
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Sounds good. Do you have commit access?


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 93403.
zhizhouy marked 2 inline comments as done.
zhizhouy added a comment.

Added testcase for recording other useful options.


https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/debug-options.c


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,18 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+// RUN: %clang -### -c -O3 -ffunction-sections -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_OPT %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +204,17 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -grecord-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
+// GRECORD_OPT: -### -c -O3 -ffunction-sections -grecord-gcc-switches
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,18 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+// RUN: %clang -### -c -O3 -ffunction-sections -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_OPT %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +204,17 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -grecord-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
+// GRECORD_OPT: -### -c -O3 -ffunction-sections -grecord-gcc-switches
+//
 // GIGNORE-NOT: "argument unused 

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo added inline comments.



Comment at: test/Driver/debug-options.c:201-202
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//

george.burgess.iv wrote:
> echristo wrote:
> > This seems a little light on the testing, would you mind adding some more 
> > interesting lines here? (Also, -grecord-gcc-switches seems like an option 
> > we might want to ignore for this?)
> re-pasting my comment, since it looks like it got dropped when we updated 
> diffs:
> 
> nit: since `RenderAsInput` args (`-Xlinker`, `-o`, ...) have cases where 
> they're printed without their arg, can we check that at least `-o -` is 
> printed sanely, as well?
How about adding things like "-O3" or "-ffunction-sections" or something? Just 
useful options that we'd expect to see.



Comment at: test/Driver/debug-options.c:209
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -greocrd-gcc-switches
+//

Typo.


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 93401.
zhizhouy marked 2 inline comments as done.
zhizhouy edited the summary of this revision.
zhizhouy added a comment.

Added two more testcases, one is options with both grecord-gcc-switches and 
gno-record-gcc-switches; the other one is testing if "-o -" is omitted 
correctly.


https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/debug-options.c


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,16 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +202,15 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -greocrd-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,16 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-gcc-switches -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +202,15 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+// GNO_RECORD-NOT: -### -c -greocrd-gcc-switches
+//
+// GRECORD_O: "-dwarf-debug-flags"
+// GRECORD_O: -### -c -grecord-gcc-switches -o -
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via cfe-commits]

Sounds good to me.

On Sun, Mar 26, 2017, 9:40 AM David Blaikie  wrote:

> Yeah, I don't know/mind either way - I think there's a tidy simplicity to
> including exactly what the user wrote on the command line, so don't mind if
> it's not removed, but can see the argument to omit it. I'd probably leave
> it in for simplicity.
>
> On Fri, Mar 24, 2017 at 5:48 PM Eric Christopher via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Not sure, that's why I asked. Is it useful? Is it one of those things we
> want to remove since it'll be common among all of the TUs that want the
> text?
>
> On Fri, Mar 24, 2017 at 3:43 PM Zhizhou Yang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Sure I can add some more tests here.
>
> For -grecord-gcc-switches itself, I think maybe we could keep it in
> recording, since it is also one of the options from command line?
>
> On Fri, Mar 24, 2017 at 2:54 PM, Eric Christopher via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> echristo added inline comments.
>
>
> 
> Comment at: test/Driver/debug-options.c:201-202
>  //
> +// GRECORD: "-dwarf-debug-flags"
> +// GRECORD: -### -c -grecord-gcc-switches
> +//
> 
> This seems a little light on the testing, would you mind adding some more
> interesting lines here? (Also, -grecord-gcc-switches seems like an option
> we might want to ignore for this?)
>
>
> https://reviews.llvm.org/D30760
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[David Blaikie via cfe-commits]

Yeah, I don't know/mind either way - I think there's a tidy simplicity to
including exactly what the user wrote on the command line, so don't mind if
it's not removed, but can see the argument to omit it. I'd probably leave
it in for simplicity.

On Fri, Mar 24, 2017 at 5:48 PM Eric Christopher via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Not sure, that's why I asked. Is it useful? Is it one of those things we
> want to remove since it'll be common among all of the TUs that want the
> text?
>
> On Fri, Mar 24, 2017 at 3:43 PM Zhizhou Yang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Sure I can add some more tests here.
>
> For -grecord-gcc-switches itself, I think maybe we could keep it in
> recording, since it is also one of the options from command line?
>
> On Fri, Mar 24, 2017 at 2:54 PM, Eric Christopher via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> echristo added inline comments.
>
>
> 
> Comment at: test/Driver/debug-options.c:201-202
>  //
> +// GRECORD: "-dwarf-debug-flags"
> +// GRECORD: -### -c -grecord-gcc-switches
> +//
> 
> This seems a little light on the testing, would you mind adding some more
> interesting lines here? (Also, -grecord-gcc-switches seems like an option
> we might want to ignore for this?)
>
>
> https://reviews.llvm.org/D30760
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via cfe-commits]

Not sure, that's why I asked. Is it useful? Is it one of those things we
want to remove since it'll be common among all of the TUs that want the
text?

On Fri, Mar 24, 2017 at 3:43 PM Zhizhou Yang via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Sure I can add some more tests here.
>
> For -grecord-gcc-switches itself, I think maybe we could keep it in
> recording, since it is also one of the options from command line?
>
> On Fri, Mar 24, 2017 at 2:54 PM, Eric Christopher via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> echristo added inline comments.
>
>
> 
> Comment at: test/Driver/debug-options.c:201-202
>  //
> +// GRECORD: "-dwarf-debug-flags"
> +// GRECORD: -### -c -grecord-gcc-switches
> +//
> 
> This seems a little light on the testing, would you mind adding some more
> interesting lines here? (Also, -grecord-gcc-switches seems like an option
> we might want to ignore for this?)
>
>
> https://reviews.llvm.org/D30760
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via cfe-commits]

Sure I can add some more tests here.

For -grecord-gcc-switches itself, I think maybe we could keep it in
recording, since it is also one of the options from command line?

On Fri, Mar 24, 2017 at 2:54 PM, Eric Christopher via Phabricator <
revi...@reviews.llvm.org> wrote:

> echristo added inline comments.
>
>
> 
> Comment at: test/Driver/debug-options.c:201-202
>  //
> +// GRECORD: "-dwarf-debug-flags"
> +// GRECORD: -### -c -grecord-gcc-switches
> +//
> 
> This seems a little light on the testing, would you mind adding some more
> interesting lines here? (Also, -grecord-gcc-switches seems like an option
> we might want to ignore for this?)
>
>
> https://reviews.llvm.org/D30760
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added inline comments.



Comment at: test/Driver/debug-options.c:201
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches

echristo wrote:
> This seems a little light on the testing, would you mind adding some more 
> interesting lines here? (Also, -grecord-gcc-switches seems like an option we 
> might want to ignore for this?)
re-pasting my comment, since it looks like it got dropped when we updated diffs:

nit: since `RenderAsInput` args (`-Xlinker`, `-o`, ...) have cases where 
they're printed without their arg, can we check that at least `-o -` is printed 
sanely, as well?


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo added inline comments.



Comment at: test/Driver/debug-options.c:201-202
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//

This seems a little light on the testing, would you mind adding some more 
interesting lines here? (Also, -grecord-gcc-switches seems like an option we 
might want to ignore for this?)


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 93011.
zhizhouy added a comment.

Added tests into test/Driver/debug-options.c. Thanks.


https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/debug-options.c


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,12 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +198,11 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -117,8 +117,12 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
-// RUN: %clang -### -c -grecord-gcc-switches -gno-record-gcc-switches \
-// RUN:-gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
+// RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+//
+// RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
 // RUN: %clang -### -c -ggnu-pubnames %s 2>&1 | FileCheck -check-prefix=GOPT %s
@@ -194,6 +198,11 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
+// GNO_RECORD-NOT: "-dwarf-debug-flags"
+//
 // GIGNORE-NOT: "argument unused during compilation"
 //
 // GOPT: -generate-gnu-dwarf-pub-sections
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

[David Blaikie via cfe-commits]

As Adrian mentioned, this can probably be covered/added to an existing test
case in clang/test/Driver

On Fri, Mar 24, 2017 at 1:57 PM Zhizhou Yang via Phabricator <
revi...@reviews.llvm.org> wrote:

> zhizhouy updated this revision to Diff 93003.
> zhizhouy marked 3 inline comments as done.
> zhizhouy added a comment.
>
> Checked both grecord-gcc-swtiches and gno-record-gcc-switches.
>
> Modified testcases for it.
>
>
> https://reviews.llvm.org/D30760
>
> Files:
>   lib/Driver/ToolChains/Clang.cpp
>   test/Driver/grecord-gcc-switches.c
>
>
> Index: test/Driver/grecord-gcc-switches.c
> ===
> --- /dev/null
> +++ test/Driver/grecord-gcc-switches.c
> @@ -0,0 +1,13 @@
> +// RUN: %clang -### -g -c -grecord-gcc-switches %s 2>&1 | FileCheck
> -check-prefix=REC %s
> +// RUN: %clang -### -g -c -gno-record-gcc-switches %s 2>&1 | FileCheck
> -check-prefix=NO_REC %s
> +// RUN: %clang -### -g -c %s 2>&1 | FileCheck %s
> +int main (void) {
> +  return 0;
> +}
> +
> +// REC: "-dwarf-debug-flags"
> +// REC: -### -g -c -grecord-gcc-switches
> +// NO_REC-NOT: "-dwarf-debug-flags"
> +// NO_REC-NOT: -### -g -c -gno-record-gcc-switches
> +// CHECK-NOT: "-dwarf-debug-flags"
> +// CHECK-NOT: -### -g -c
> Index: lib/Driver/ToolChains/Clang.cpp
> ===
> --- lib/Driver/ToolChains/Clang.cpp
> +++ lib/Driver/ToolChains/Clang.cpp
> @@ -2728,7 +2728,8 @@
>  DwarfVersion = getToolChain().GetDefaultDwarfVersion();
>}
>
> -  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
> +  // We ignore flag -gstrict-dwarf for now.
> +  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
>Args.ClaimAllArgs(options::OPT_g_flags_Group);
>
>// Column info is included by default for everything except PS4 and
> CodeView.
> @@ -4321,7 +4322,12 @@
>
>// Optionally embed the -cc1 level arguments into the debug info, for
> build
>// analysis.
> -  if (getToolChain().UseDwarfDebugFlags()) {
> +  // Also record command line arguments into the debug info if
> +  // -grecord-gcc-switches options is set on.
> +  // By default, -gno-record-gcc-switches is set on and no recording.
> +  if (getToolChain().UseDwarfDebugFlags() ||
> +  Args.hasFlag(options::OPT_grecord_gcc_switches,
> +   options::OPT_gno_record_gcc_switches, false)) {
>  ArgStringList OriginalArgs;
>  for (const auto  : Args)
>Arg->render(Args, OriginalArgs);
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 93003.
zhizhouy marked 3 inline comments as done.
zhizhouy added a comment.

Checked both grecord-gcc-swtiches and gno-record-gcc-switches.

Modified testcases for it.


https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/grecord-gcc-switches.c


Index: test/Driver/grecord-gcc-switches.c
===
--- /dev/null
+++ test/Driver/grecord-gcc-switches.c
@@ -0,0 +1,13 @@
+// RUN: %clang -### -g -c -grecord-gcc-switches %s 2>&1 | FileCheck 
-check-prefix=REC %s
+// RUN: %clang -### -g -c -gno-record-gcc-switches %s 2>&1 | FileCheck 
-check-prefix=NO_REC %s
+// RUN: %clang -### -g -c %s 2>&1 | FileCheck %s
+int main (void) {
+  return 0;
+}
+
+// REC: "-dwarf-debug-flags"
+// REC: -### -g -c -grecord-gcc-switches
+// NO_REC-NOT: "-dwarf-debug-flags"
+// NO_REC-NOT: -### -g -c -gno-record-gcc-switches
+// CHECK-NOT: "-dwarf-debug-flags"
+// CHECK-NOT: -### -g -c
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);


Index: test/Driver/grecord-gcc-switches.c
===
--- /dev/null
+++ test/Driver/grecord-gcc-switches.c
@@ -0,0 +1,13 @@
+// RUN: %clang -### -g -c -grecord-gcc-switches %s 2>&1 | FileCheck -check-prefix=REC %s
+// RUN: %clang -### -g -c -gno-record-gcc-switches %s 2>&1 | FileCheck -check-prefix=NO_REC %s
+// RUN: %clang -### -g -c %s 2>&1 | FileCheck %s
+int main (void) {
+  return 0;
+}
+
+// REC: "-dwarf-debug-flags"
+// REC: -### -g -c -grecord-gcc-switches
+// NO_REC-NOT: "-dwarf-debug-flags"
+// NO_REC-NOT: -### -g -c -gno-record-gcc-switches
+// CHECK-NOT: "-dwarf-debug-flags"
+// CHECK-NOT: -### -g -c
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,12 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  // By default, -gno-record-gcc-switches is set on and no recording.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasFlag(options::OPT_grecord_gcc_switches,
+   options::OPT_gno_record_gcc_switches, false)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: test/CodeGen/debug-info-grecord-gcc-switches.c:1
+// RUN: %clang -g -grecord-gcc-switches -S -emit-llvm -o - %s | FileCheck %s
+int main (void) {

Shouldn't this test be in the Driver/ directory? There is already a big file 
that exercises all -g options, I would just append it there.


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

Needs more testing. Might want to make sure that you actually are recording 
some useful command line options and that you're looking at the cc1 command 
line.

This should also be a Driver test and not CodeGen. You can then use -### to 
inspect the command lines as passed around.

-eric


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:4328
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasArg(options::OPT_grecord_gcc_switches)) {
 ArgStringList OriginalArgs;

looks like we have to consider `-gno-record-gcc-switches` (which already exists 
in Options.td), too. `hasFlag` should let you test them both easily.

please also add a test checking that we respect `-gno-record-gcc-switches`.



Comment at: test/CodeGen/debug-info-grecord-gcc-switches.c:6
+
+// CHECK: -g -grecord-gcc-switches -S -emit-llvm

nit: since `RenderAsInput` args (`-Xlinker`, `-o`, ...) have cases where 
they're printed without their arg, can we check that `-o -` is printed sanely 
here, as well?


https://reviews.llvm.org/D30760



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang, Clang part

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 92885.
zhizhouy retitled this revision from "Record command lines in objects built by 
clang" to "Record command lines in objects built by clang, Clang part".
zhizhouy edited the summary of this revision.
zhizhouy added a reviewer: aprantl.
zhizhouy added a comment.

I am reusing the DwarfDebugFlags to try to solve this bug now.

DwarfDebugFlags only records the command line options for Apple Darwin and will 
generate DW_AT_APPLE_flags into debug info.
I decide not to touch those parts for now.

When -grecord-gcc-switches is set, I record command line options into 
DwarfDebugFlags no matter what ToolChain currently is.

This patch also modified LLVM side, which can be referred to: name 



https://reviews.llvm.org/D30760

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGen/debug-info-grecord-gcc-switches.c


Index: test/CodeGen/debug-info-grecord-gcc-switches.c
===
--- /dev/null
+++ test/CodeGen/debug-info-grecord-gcc-switches.c
@@ -0,0 +1,6 @@
+// RUN: %clang -g -grecord-gcc-switches -S -emit-llvm -o - %s | FileCheck %s
+int main (void) {
+  return 0;
+}
+
+// CHECK: -g -grecord-gcc-switches -S -emit-llvm
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,10 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasArg(options::OPT_grecord_gcc_switches)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);


Index: test/CodeGen/debug-info-grecord-gcc-switches.c
===
--- /dev/null
+++ test/CodeGen/debug-info-grecord-gcc-switches.c
@@ -0,0 +1,6 @@
+// RUN: %clang -g -grecord-gcc-switches -S -emit-llvm -o - %s | FileCheck %s
+int main (void) {
+  return 0;
+}
+
+// CHECK: -g -grecord-gcc-switches -S -emit-llvm
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2728,7 +2728,8 @@
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();
   }
 
-  // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
+  // We ignore flag -gstrict-dwarf for now.
+  // And we handle flag -grecord-gcc-switches later with DwarfDebugFlags.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
   // Column info is included by default for everything except PS4 and CodeView.
@@ -4321,7 +4322,10 @@
 
   // Optionally embed the -cc1 level arguments into the debug info, for build
   // analysis.
-  if (getToolChain().UseDwarfDebugFlags()) {
+  // Also record command line arguments into the debug info if
+  // -grecord-gcc-switches options is set on.
+  if (getToolChain().UseDwarfDebugFlags() ||
+  Args.hasArg(options::OPT_grecord_gcc_switches)) {
 ArgStringList OriginalArgs;
 for (const auto  : Args)
   Arg->render(Args, OriginalArgs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits