[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:980-981
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

garvitgupta08 wrote:
> nickdesaulniers wrote:
> > The following input causes clang to crash:
> > 
> > ```
> > $ clang -fno-integrated-as -c -x assembler-with-cpp /dev/null
> > clang: /android0/llvm-project/llvm/include/llvm/ADT/StringRef.h:597: 
> > llvm::StringRef llvm::StringRef::drop_front(size_t) const: Assertion 
> > `size() >= N && "Dropping more elements than exist"' failed.
> > ```
> > 
> > Please fix that and add a test case for `/dev/null` (i.e. inputs for which 
> > no extension exists).
> For Inputs without extension, will there always be a -x flag?
I assume the input can have any extension or no extension. Maybe find where in 
clang we decide we don't know the extension, and emulate a similar approach?

For example, `clang foo.xyz` will complain that we don't know the source 
language. `clang -x c foo.xyz` should shut up the warning if `foo.xyz` is a 
valid c file.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:980-981
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

nickdesaulniers wrote:
> The following input causes clang to crash:
> 
> ```
> $ clang -fno-integrated-as -c -x assembler-with-cpp /dev/null
> clang: /android0/llvm-project/llvm/include/llvm/ADT/StringRef.h:597: 
> llvm::StringRef llvm::StringRef::drop_front(size_t) const: Assertion `size() 
> >= N && "Dropping more elements than exist"' failed.
> ```
> 
> Please fix that and add a test case for `/dev/null` (i.e. inputs for which no 
> extension exists).
For Inputs without extension, will there always be a -x flag?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

In D145726#4228406 , @MaskRay wrote:

> Please update the commit message and comment about what binutils versions 
> reject the construct.

I already added this - "Due to this we started seeing
below assembler error with GNU binutils version 2.34 and below. This
error is not shown by binutils version 2.36 and later."


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers requested changes to this revision.
nickdesaulniers added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:980-981
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

The following input causes clang to crash:

```
$ clang -fno-integrated-as -c -x assembler-with-cpp /dev/null
clang: /android0/llvm-project/llvm/include/llvm/ADT/StringRef.h:597: 
llvm::StringRef llvm::StringRef::drop_front(size_t) const: Assertion `size() >= 
N && "Dropping more elements than exist"' failed.
```

Please fix that and add a test case for `/dev/null` (i.e. inputs for which no 
extension exists).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Please update the commit message and comment about what binutils versions 
reject the construct.




Comment at: clang/test/Driver/gcc_forward.c:49
+// DEBUG-NOT: "-gdwarf-4"
\ No newline at end of file


No newline at end of file

Please fix


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

In D145726#4228216 , @nickdesaulniers 
wrote:

> @garvitgupta08 what's your email address so that I may attribute your 
> authorship correctly when committing on your behalf?

quic_garvg...@quicinc.com. Thanks for committing!.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

@garvitgupta08 what's your email address so that I may attribute your 
authorship correctly when committing on your behalf?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D145726#4225082 , @garvitgupta08 
wrote:

> I do not have the commit access, can you commit on my behalf @nickdesaulniers

Sure thing; I'll test it out with some Linux kernel builds, too.  I'll try to 
get that done today by EOD; thanks again for the patch!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

I do not have the commit access, can you commit on my behalf @nickdesaulniers


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:976-985
+  bool IsInputTyAsm = false;
+  for (const auto  : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

nickdesaulniers wrote:
> garvitgupta08 wrote:
> > nickdesaulniers wrote:
> > > Thinking about this more, does the issue still exist if the user passed 
> > > .c and .s/.S files together?
> > > 
> > > i.e. `$ clang ... -fno-integrated-as -gdwarf-4 foo.s main.c`?
> > Yes, the error will still be thrown for c/cpp files.
> So this patch is an incomplete fix then? Is there somewhere else we can move 
> this logic then so that it's only applied for individual files and not 
> multiple inputs?
No. I misunderstood your question. I thought you were asking what will happen 
in case of multiple inputs without this fix. I answered according to this. 

The current patch fixes the error in case of multiple inputs as well. No error 
will be thrown for c/cpp files. I hope this answers your original question.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:976-985
+  bool IsInputTyAsm = false;
+  for (const auto  : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

garvitgupta08 wrote:
> nickdesaulniers wrote:
> > Thinking about this more, does the issue still exist if the user passed .c 
> > and .s/.S files together?
> > 
> > i.e. `$ clang ... -fno-integrated-as -gdwarf-4 foo.s main.c`?
> Yes, the error will still be thrown for c/cpp files.
So this patch is an incomplete fix then? Is there somewhere else we can move 
this logic then so that it's only applied for individual files and not multiple 
inputs?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:976-985
+  bool IsInputTyAsm = false;
+  for (const auto  : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

nickdesaulniers wrote:
> Thinking about this more, does the issue still exist if the user passed .c 
> and .s/.S files together?
> 
> i.e. `$ clang ... -fno-integrated-as -gdwarf-4 foo.s main.c`?
Yes, the error will still be thrown for c/cpp files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 updated this revision to Diff 508328.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/as-options.cpp
  clang/test/Driver/gcc_forward.c


Index: clang/test/Driver/gcc_forward.c
===
--- clang/test/Driver/gcc_forward.c
+++ clang/test/Driver/gcc_forward.c
@@ -34,3 +34,15 @@
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
+//
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
\ No newline at end of file
Index: clang/test/Driver/as-options.cpp
===
--- /dev/null
+++ clang/test/Driver/as-options.cpp
@@ -0,0 +1,11 @@
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -972,19 +972,27 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  for (const auto  : Inputs)
-CmdArgs.push_back(II.getFilename());
-
-  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
-   options::OPT_gdwarf_2, options::OPT_gdwarf_3,
-   options::OPT_gdwarf_4, options::OPT_gdwarf_5,
-   options::OPT_gdwarf))
-if (!A->getOption().matches(options::OPT_g0)) {
-  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
 
-  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
-  CmdArgs.push_back(Args.MakeArgString("-gdwarf-" + Twine(DwarfVersion)));
-}
+  bool IsInputTyAsm = false;
+  for (const auto  : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)
+  IsInputTyAsm = true;
+  }
+
+  if (IsInputTyAsm)
+if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+ options::OPT_gdwarf_2, options::OPT_gdwarf_3,
+ options::OPT_gdwarf_4, options::OPT_gdwarf_5,
+ options::OPT_gdwarf))
+  if (!A->getOption().matches(options::OPT_g0)) {
+Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
+CmdArgs.push_back(Args.MakeArgString("-gdwarf-" + 
Twine(DwarfVersion)));
+  }
 
   const char *Exec =
   Args.MakeArgString(getToolChain().GetProgramPath(DefaultAssembler));


Index: clang/test/Driver/gcc_forward.c
===
--- clang/test/Driver/gcc_forward.c
+++ clang/test/Driver/gcc_forward.c
@@ -34,3 +34,15 @@
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
+//
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:976-985
+  bool IsInputTyAsm = false;
+  for (const auto  : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

Thinking about this more, does the issue still exist if the user passed .c and 
.s/.S files together?

i.e. `$ clang ... -fno-integrated-as -gdwarf-4 foo.s main.c`?



Comment at: clang/test/Driver/as-options.cpp:1
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \

please `chmod 644` this file; `755` is what I'd expect for a directory.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:978-986
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > garvitgupta08 wrote:
> > > nickdesaulniers wrote:
> > > > Isn't this potentially going to add `-gdwarf-` repeatedly if there's 
> > > > many inputs?
> > > > 
> > > > Wouldn't it be better to scan the inputs to see if there's any .S or .s 
> > > > files, then add the flags once?
> > > Let me know if this is fine.
> > I don't think the current implementation addresses my point.  Having 
> > `CmdArgs.push_back` be called in a loop on the number of inputs will 
> > potentially add the arg repeatedly.
> > 
> > I think you should simply check if the `InputType` is asm or asm-with-cpp 
> > in the loop, potentially setting a boolean scoped outside the loop. Then, 
> > after the loop, decide whether to add the cmd arg.
> Specifically, if `llvm::any_of` the inputs are asm or asm-with-cpp, then we 
> might want to modify the command line flags passed to the external assembler.
> 
> We don't want to pass additional flags per input.
Done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

In D145726#4194142 , @nickdesaulniers 
wrote:

> The description needs to be rewritten to elucidate that this is purely to 
> support old versions of the GNU assembler still in use by various Linux LTS 
> distros.  This problem does not exist for folks using up to date tooling.  I 
> would like to see specific GNU assembler version details in the commit 
> description as well.

Done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 updated this revision to Diff 507144.
garvitgupta08 marked an inline comment as not done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/as-options.cpp
  clang/test/Driver/gcc_forward.c


Index: clang/test/Driver/gcc_forward.c
===
--- clang/test/Driver/gcc_forward.c
+++ clang/test/Driver/gcc_forward.c
@@ -34,3 +34,15 @@
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
+//
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
\ No newline at end of file
Index: clang/test/Driver/as-options.cpp
===
--- /dev/null
+++ clang/test/Driver/as-options.cpp
@@ -0,0 +1,11 @@
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -972,19 +972,27 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  for (const auto  : Inputs)
-CmdArgs.push_back(II.getFilename());
-
-  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
-   options::OPT_gdwarf_2, options::OPT_gdwarf_3,
-   options::OPT_gdwarf_4, options::OPT_gdwarf_5,
-   options::OPT_gdwarf))
-if (!A->getOption().matches(options::OPT_g0)) {
-  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
 
-  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
-  CmdArgs.push_back(Args.MakeArgString("-gdwarf-" + Twine(DwarfVersion)));
-}
+  bool IsInputTyAsm = false;
+  for (const auto  : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)
+  IsInputTyAsm = true;
+  }
+
+  if (IsInputTyAsm)
+if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+ options::OPT_gdwarf_2, options::OPT_gdwarf_3,
+ options::OPT_gdwarf_4, options::OPT_gdwarf_5,
+ options::OPT_gdwarf))
+  if (!A->getOption().matches(options::OPT_g0)) {
+Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
+CmdArgs.push_back(Args.MakeArgString("-gdwarf-" + 
Twine(DwarfVersion)));
+  }
 
   const char *Exec =
   Args.MakeArgString(getToolChain().GetProgramPath(DefaultAssembler));


Index: clang/test/Driver/gcc_forward.c
===
--- clang/test/Driver/gcc_forward.c
+++ clang/test/Driver/gcc_forward.c
@@ -34,3 +34,15 @@
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
+//
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang 

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:978-986
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);

nickdesaulniers wrote:
> garvitgupta08 wrote:
> > nickdesaulniers wrote:
> > > Isn't this potentially going to add `-gdwarf-` repeatedly if there's many 
> > > inputs?
> > > 
> > > Wouldn't it be better to scan the inputs to see if there's any .S or .s 
> > > files, then add the flags once?
> > Let me know if this is fine.
> I don't think the current implementation addresses my point.  Having 
> `CmdArgs.push_back` be called in a loop on the number of inputs will 
> potentially add the arg repeatedly.
> 
> I think you should simply check if the `InputType` is asm or asm-with-cpp in 
> the loop, potentially setting a boolean scoped outside the loop. Then, after 
> the loop, decide whether to add the cmd arg.
Specifically, if `llvm::any_of` the inputs are asm or asm-with-cpp, then we 
might want to modify the command line flags passed to the external assembler.

We don't want to pass additional flags per input.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers requested changes to this revision.
nickdesaulniers added a comment.
This revision now requires changes to proceed.

The description needs to be rewritten to elucidate that this is purely to 
support old versions of the GNU assembler still in use by various Linux LTS 
distros.  This problem does not exist for folks using up to date tooling.  I 
would like to see specific GNU assembler version details in the commit 
description as well.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:978-986
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);

garvitgupta08 wrote:
> nickdesaulniers wrote:
> > Isn't this potentially going to add `-gdwarf-` repeatedly if there's many 
> > inputs?
> > 
> > Wouldn't it be better to scan the inputs to see if there's any .S or .s 
> > files, then add the flags once?
> Let me know if this is fine.
I don't think the current implementation addresses my point.  Having 
`CmdArgs.push_back` be called in a loop on the number of inputs will 
potentially add the arg repeatedly.

I think you should simply check if the `InputType` is asm or asm-with-cpp in 
the loop, potentially setting a boolean scoped outside the loop. Then, after 
the loop, decide whether to add the cmd arg.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.






Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:978-986
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);

nickdesaulniers wrote:
> Isn't this potentially going to add `-gdwarf-` repeatedly if there's many 
> inputs?
> 
> Wouldn't it be better to scan the inputs to see if there's any .S or .s 
> files, then add the flags once?
Let me know if this is fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 updated this revision to Diff 505171.
garvitgupta08 added a comment.

Addressed the comments. Using gcc_forward.c for checking c extension.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/as-options.cpp
  clang/test/Driver/gcc_forward.c


Index: clang/test/Driver/gcc_forward.c
===
--- clang/test/Driver/gcc_forward.c
+++ clang/test/Driver/gcc_forward.c
@@ -34,3 +34,15 @@
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
+//
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
Index: clang/test/Driver/as-options.cpp
===
--- /dev/null
+++ clang/test/Driver/as-options.cpp
@@ -0,0 +1,11 @@
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -972,19 +972,24 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  for (const auto  : Inputs)
+  for (const auto  : Inputs) {
 CmdArgs.push_back(II.getFilename());
-
-  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
-   options::OPT_gdwarf_2, options::OPT_gdwarf_3,
-   options::OPT_gdwarf_4, options::OPT_gdwarf_5,
-   options::OPT_gdwarf))
-if (!A->getOption().matches(options::OPT_g0)) {
-  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
-
-  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
-  CmdArgs.push_back(Args.MakeArgString("-gdwarf-" + Twine(DwarfVersion)));
-}
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
+  CmdArgs.push_back(
+  Args.MakeArgString("-gdwarf-" + Twine(DwarfVersion)));
+  break;
+}
+  }
 
   const char *Exec =
   Args.MakeArgString(getToolChain().GetProgramPath(DefaultAssembler));


Index: clang/test/Driver/gcc_forward.c
===
--- clang/test/Driver/gcc_forward.c
+++ clang/test/Driver/gcc_forward.c
@@ -34,3 +34,15 @@
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
+//
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D145726#4190341 , @garvitgupta08 
wrote:

> In D145726#4190316 , 
> @nickdesaulniers wrote:
>
>> Isn't this only an issue with ancient versions of GNU as? Older than 2.16?
>
> I am able to reproduce this issue with 4.9.0 GNU as.

binutils has a separate version from gcc.

Tried the following file with binutils on godbolt; apparently binutils 2.34 and 
earlier reject, binutils 2.36 and later accept. So we're not talking about the 
most recent binutils, but 2.34 isn't exactly ancient.

  nop
  .file 1 "asdf"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:978-986
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);

Isn't this potentially going to add `-gdwarf-` repeatedly if there's many 
inputs?

Wouldn't it be better to scan the inputs to see if there's any .S or .s files, 
then add the flags once?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D145726#4190341 , @garvitgupta08 
wrote:

> In D145726#4190316 , 
> @nickdesaulniers wrote:
>
>> Isn't this only an issue with ancient versions of GNU as? Older than 2.16?
>
> I am able to reproduce this issue with 4.9.0 GNU as.

That version number looks like a GCC version number.

GAS should be 2.39 or close by. Please run `as --version`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

In D145726#4190316 , @nickdesaulniers 
wrote:

> Isn't this only an issue with ancient versions of GNU as? Older than 2.16?

I am able to reproduce this issue with 4.9.0 GNU as.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Rather than create new test files, shouldn't this modify the same tests 
modified in D136309  and D136707 
?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

Added the testcase in the commit  message itself. Thanks Eli for providing the 
same!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Isn't this only an issue with ancient versions of GNU as? Older than 2.16?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Following reproduces for me (clang from main, Ubuntu 16.04).

  $ cat test.cpp
  int foo()
  {
  int i=6;
  do --i; while (!(i%3));
  do {} while (!(i%5));
  return 0;
  }
  $ clang++ test.cpp -c -fno-integrated-as -gdwarf-4 -O2 -fno-finite-loops
  /tmp/test-f97496.s: Assembler messages:
  /tmp/test-f97496.s:18: Error: file number 1 already allocated
  clang++: error: assembler command failed with exit code 1 (use -v to see 
invocation)

I think it triggers when the assembly file contains code before the first 
".file" directive.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> Due to this we started seeing assembler errors with certain .c and .cpp files 
> -
> "Error: file number 1 already allocated"

What are the certain `.c` and `.cpp` files? The behavior is correct for the 
following two commands.

  clang --target=arm-linux-gnueabihf -fno-integrated-as -c a.cc
  clang --target=arm-linux-gnueabihf -fno-integrated-as -S a.c

(I'll be out for most of the next 4 weeks and will spend little time on 
reviews.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145726/new/

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 created this revision.
garvitgupta08 added reviewers: apazos, efriedma, MaskRay, nickdesaulniers.
Herald added a project: All.
garvitgupta08 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D136309  and D136707 
 are 2 differentials that pass -g and 
-gdwarf-* to
assembler when -fno-integrated-as is used. Due to this we started seeing
assembler errors with certain .c and .cpp files -

"Error: file number 1 already allocated"

This is because the debug info generated at the source code level is conflicting
with the debug info generated by assembler as mentioned in the gcc bug report -
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35925

This patch solves the above failure by passing -g and -gdwarf-* flags to
assembler only when the source code is assembly, otherwise just generate the
debug info at the source code level.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145726

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/as-options.c
  clang/test/Driver/as-options.cpp


Index: clang/test/Driver/as-options.cpp
===
--- /dev/null
+++ clang/test/Driver/as-options.cpp
@@ -0,0 +1,11 @@
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
Index: clang/test/Driver/as-options.c
===
--- /dev/null
+++ clang/test/Driver/as-options.c
@@ -0,0 +1,11 @@
+// Test that -g and -gdwarf-* are not passed through to GAS.
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g -c %s -### 
2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -c %s 
-### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -gdwarf-4 -g -c 
%s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=arm-linux-gnueabi -fno-integrated-as -g \
+// RUN:   -fdebug-default-version=4 -c %s -### 2>&1 | FileCheck 
--check-prefix=DEBUG %s
+// DEBUG-NOT: "-g"
+// DEBUG-NOT: "-gdwarf-4"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -969,19 +969,23 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  for (const auto  : Inputs)
+  for (const auto  : Inputs) {
 CmdArgs.push_back(II.getFilename());
-
-  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
-   options::OPT_gdwarf_2, options::OPT_gdwarf_3,
-   options::OPT_gdwarf_4, options::OPT_gdwarf_5,
-   options::OPT_gdwarf))
-if (!A->getOption().matches(options::OPT_g0)) {
-  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
-
-  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
-  CmdArgs.push_back(Args.MakeArgString("-gdwarf-" + Twine(DwarfVersion)));
-}
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group,
+   options::OPT_gdwarf_2, 
options::OPT_gdwarf_3,
+   options::OPT_gdwarf_4, 
options::OPT_gdwarf_5,
+   options::OPT_gdwarf))
+if (!A->getOption().matches(options::OPT_g0)) {
+  Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+  unsigned DwarfVersion = getDwarfVersion(getToolChain(), Args);
+  CmdArgs.push_back(
+  Args.MakeArgString("-gdwarf-" + Twine(DwarfVersion)));
+}
+  }
 
   const char *Exec =
   Args.MakeArgString(getToolChain().GetProgramPath(DefaultAssembler));


Index: clang/test/Driver/as-options.cpp
===
--- /dev/null
+++ clang/test/Driver/as-options.cpp
@@ -0,0 +1,11 @@
+// Test that -g and -gdwarf-* are not passed through to