[PATCH] D49589: [UBSan] Strengthen pointer checks in 'new' expressions

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

You may want to add a test for placement new, no?


Repository:
  rC Clang

https://reviews.llvm.org/D49589



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

[PATCH] D49589: [UBSan] Strengthen pointer checks in 'new' expressions

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Did you consider dividing the patch into two separate changes to solve the two 
described issues independently?


Repository:
  rC Clang

https://reviews.llvm.org/D49589



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

[PATCH] D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: phosek, Hahnfeld, logan, rnk.
ikudrin added a project: clang.

Right now, we have to add an `.exe` suffix when using this switch, like 
`-fuse-ld=lld.exe`.
If the suffix is omitted, `llvm::sys::fs::exists()` cannot find the file on 
Windows,
while `llvm::sys::fs::can_Execute()` automatically tries both variants.


Repository:
  rC Clang

https://reviews.llvm.org/D43621

Files:
  lib/Driver/ToolChain.cpp


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -412,7 +412,7 @@
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
 // If we're passed -fuse-ld= with no argument, or with the argument ld,
@@ -427,7 +427,7 @@
 LinkerName.append(UseLinker);
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
+if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
   }
 


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -412,7 +412,7 @@
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
 // If we're passed -fuse-ld= with no argument, or with the argument ld,
@@ -427,7 +427,7 @@
 LinkerName.append(UseLinker);
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
+if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added inline comments.



Comment at: lib/Driver/ToolChain.cpp:415
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;

Hahnfeld wrote:
> I don't think we should do this for absolute paths?
Why not? You can omit ".exe" suffix on Windows when calling an application, 
right?


Repository:
  rC Clang

https://reviews.llvm.org/D43621



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

[PATCH] D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Not all toolchains call `ToolChain::GetLinkerPath`. For example, MSVC toolchain 
uses its own code:

  void visualstudio::Linker::ConstructJob(...) {
  ...
StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "link");
if (Linker.equals_lower("lld"))
  Linker = "lld-link";
  ...
  }

In my case, I am trying to cross-compile:

  > ...\clang.exe a.cpp -fuse-ld=lld -target i686-pc-linux-gnu
  clang.exe: error: invalid linker name in argument '-fuse-ld=lld'


Repository:
  rC Clang

https://reviews.llvm.org/D43621



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

[PATCH] D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin updated this revision to Diff 135888.
ikudrin added a comment.

- Add a test.


https://reviews.llvm.org/D43621

Files:
  lib/Driver/ToolChain.cpp
  test/Driver/Inputs/fuse_ld_windows/ld.foo.exe
  test/Driver/fuse-ld-windows.c


Index: test/Driver/fuse-ld-windows.c
===
--- /dev/null
+++ test/Driver/fuse-ld-windows.c
@@ -0,0 +1,25 @@
+// REQUIRES: system-windows
+
+// We used to require adding ".exe" suffix when cross-compiling on Windows.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the old variant still works.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// With the full path, the extension can be omitted, too,
+// because Windows allows that.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the full path with the extension works too.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-NOT: invalid linker name
+// CHECK: /Inputs/fuse_ld_windows{{/|}}ld.foo
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -412,7 +412,7 @@
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
 // If we're passed -fuse-ld= with no argument, or with the argument ld,
@@ -427,7 +427,7 @@
 LinkerName.append(UseLinker);
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
+if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
   }
 


Index: test/Driver/fuse-ld-windows.c
===
--- /dev/null
+++ test/Driver/fuse-ld-windows.c
@@ -0,0 +1,25 @@
+// REQUIRES: system-windows
+
+// We used to require adding ".exe" suffix when cross-compiling on Windows.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the old variant still works.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// With the full path, the extension can be omitted, too,
+// because Windows allows that.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the full path with the extension works too.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-NOT: invalid linker name
+// CHECK: /Inputs/fuse_ld_windows{{/|}}ld.foo
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -412,7 +412,7 @@
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
 // If we're passed -fuse-ld= with no argument, or with the argument ld,
@@ -427,7 +427,7 @@
 LinkerName.append(UseLinker);
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
+if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326164: [Driver] Allow using a canonical form of 
'-fuse-ld=' when cross-compiling on… (authored by ikudrin, committed 
by ).

Changed prior to commit:
  https://reviews.llvm.org/D43621?vs=135888&id=136026#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43621

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/test/Driver/Inputs/fuse_ld_windows/ld.foo.exe
  cfe/trunk/test/Driver/fuse-ld-windows.c


Index: cfe/trunk/test/Driver/fuse-ld-windows.c
===
--- cfe/trunk/test/Driver/fuse-ld-windows.c
+++ cfe/trunk/test/Driver/fuse-ld-windows.c
@@ -0,0 +1,25 @@
+// REQUIRES: system-windows
+
+// We used to require adding ".exe" suffix when cross-compiling on Windows.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the old variant still works.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// With the full path, the extension can be omitted, too,
+// because Windows allows that.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the full path with the extension works too.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-NOT: invalid linker name
+// CHECK: /Inputs/fuse_ld_windows{{/|}}ld.foo
Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -412,7 +412,7 @@
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
 // If we're passed -fuse-ld= with no argument, or with the argument ld,
@@ -427,7 +427,7 @@
 LinkerName.append(UseLinker);
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
+if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
   }
 


Index: cfe/trunk/test/Driver/fuse-ld-windows.c
===
--- cfe/trunk/test/Driver/fuse-ld-windows.c
+++ cfe/trunk/test/Driver/fuse-ld-windows.c
@@ -0,0 +1,25 @@
+// REQUIRES: system-windows
+
+// We used to require adding ".exe" suffix when cross-compiling on Windows.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the old variant still works.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -B %S/Inputs/fuse_ld_windows -fuse-ld=foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// With the full path, the extension can be omitted, too,
+// because Windows allows that.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the full path with the extension works too.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN: -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-NOT: invalid linker name
+// CHECK: /Inputs/fuse_ld_windows{{/|}}ld.foo
Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -412,7 +412,7 @@
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
 // second-guess that.
-if (llvm::sys::fs::exists(UseLinker))
+if (llvm::sys::fs::can_execute(UseLinker))
   return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
 // If we're passed -fuse-ld= with no argument, or with the argument ld,
@@ -427,7 +427,7 @@
 LinkerName.append(UseLinker);
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
+if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48531: [CodeGen] Provide source locations for UBSan type checks when emitting constructor calls.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: rsmith, rjmccall.
ikudrin added a project: clang.
Herald added subscribers: JDevlieghere, aprantl.

At the moment. when generating UBSan diagnostics for these cases, we rely on
the corresponding debug information, which might be absent, and, anyway,
not that accurate.


Repository:
  rC Clang

https://reviews.llvm.org/D48531

Files:
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenCXX/ubsan-ctor-srcloc.cpp

Index: test/CodeGenCXX/ubsan-ctor-srcloc.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-ctor-srcloc.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux -emit-llvm -fsanitize=alignment -fblocks %s -o %t.ll
+// RUN: FileCheck -check-prefix=ZEROINIT < %t.ll %s
+// RUN: FileCheck -check-prefix=SRCLOC < %t.ll %s
+
+// ZEROINIT-NOT: @{{.+}} = private unnamed_addr global {{.+}} zeroinitializer
+struct A {
+  A(int);
+  int k;
+};
+
+struct B : A {
+  B();
+  B(const B &);
+// SRCLOC-DAG: @{{.+}} = private unnamed_addr global {{.+}} @.src, i32 [[@LINE+1]], i32 12 }
+  using A::A;
+  void f() const;
+};
+
+// SRCLOC-DAG: @{{.+}} = private unnamed_addr global {{.+}} @.src, i32 [[@LINE+1]], i32 10 }
+B::B() : A(1) {}
+
+void foo() {
+  B b(2);
+// SRCLOC-DAG: @{{.+}} = private unnamed_addr global {{.+}} @.src, i32 [[@LINE+1]], i32 5 }
+  ^{b.f();}();
+}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2362,7 +2362,8 @@
   void EmitCXXConstructorCall(const CXXConstructorDecl *D, CXXCtorType Type,
   bool ForVirtualBase, bool Delegating,
   Address This, CallArgList &Args,
-  AggValueSlot::Overlap_t Overlap);
+  AggValueSlot::Overlap_t Overlap,
+  SourceLocation Loc);
 
   /// Emit assumption load for all bases. Requires to be be called only on
   /// most-derived class and not under construction of the object.
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2031,7 +2031,7 @@
/*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args,
- Overlap);
+ Overlap, E->getExprLoc());
 }
 
 static bool canEmitDelegateCallArgs(CodeGenFunction &CGF,
@@ -2064,14 +2064,14 @@
  bool Delegating,
  Address This,
  CallArgList &Args,
- AggValueSlot::Overlap_t Overlap) {
+ AggValueSlot::Overlap_t Overlap,
+ SourceLocation Loc) {
   const CXXRecordDecl *ClassDecl = D->getParent();
 
   // C++11 [class.mfct.non-static]p2:
   //   If a non-static member function of a class X is called for an object that
   //   is not of type X, or of a type derived from X, the behavior is undefined.
-  // FIXME: Provide a source location here.
-  EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, SourceLocation(),
+  EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, Loc,
 This.getPointer(), getContext().getRecordType(ClassDecl));
 
   if (D->isTrivial() && D->isDefaultConstructor()) {
@@ -2180,7 +2180,8 @@
   }
 
   EmitCXXConstructorCall(D, Ctor_Base, ForVirtualBase, /*Delegating*/false,
- This, Args, AggValueSlot::MayOverlap);
+ This, Args, AggValueSlot::MayOverlap,
+ E->getLocation());
 }
 
 void CodeGenFunction::EmitInlinedInheritingCXXConstructorCall(
@@ -2277,7 +2278,7 @@
/*ParamsToSkip*/ 1);
 
   EmitCXXConstructorCall(D, Ctor_Complete, false, false, This, Args,
- AggValueSlot::MayOverlap);
+ AggValueSlot::MayOverlap, E->getExprLoc());
 }
 
 void
@@ -2313,7 +2314,7 @@
 
   EmitCXXConstructorCall(Ctor, CtorType, /*ForVirtualBase=*/false,
  /*Delegating=*/true, This, DelegateArgs,
- AggValueSlot::MayOverlap);
+ AggValueSlot::MayOverlap, Loc);
 }
 
 namespace {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48531: [CodeGen] Provide source locations for UBSan type checks when emitting constructor calls.

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL335445: [CodeGen] Provide source locations for UBSan type 
checks when emitting… (authored by ikudrin, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48531?vs=152625&id=152626#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48531

Files:
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/test/CodeGenCXX/ubsan-ctor-srcloc.cpp

Index: cfe/trunk/test/CodeGenCXX/ubsan-ctor-srcloc.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-ctor-srcloc.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-ctor-srcloc.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux -emit-llvm -fsanitize=alignment -fblocks %s -o %t.ll
+// RUN: FileCheck -check-prefix=ZEROINIT < %t.ll %s
+// RUN: FileCheck -check-prefix=SRCLOC < %t.ll %s
+// ZEROINIT-NOT: @{{.+}} = private unnamed_addr global {{.+}} zeroinitializer
+
+struct A {
+  A(int);
+  int k;
+};
+
+struct B : A {
+  B();
+  B(const B &);
+// SRCLOC-DAG: @{{.+}} = private unnamed_addr global {{.+}} @.src, i32 [[@LINE+1]], i32 12 }
+  using A::A;
+  void f() const;
+};
+
+// SRCLOC-DAG: @{{.+}} = private unnamed_addr global {{.+}} @.src, i32 [[@LINE+1]], i32 10 }
+B::B() : A(1) {}
+
+void foo() {
+  B b(2);
+// SRCLOC-DAG: @{{.+}} = private unnamed_addr global {{.+}} @.src, i32 [[@LINE+1]], i32 5 }
+  ^{b.f();}();
+}
Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -2031,7 +2031,7 @@
/*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args,
- Overlap);
+ Overlap, E->getExprLoc());
 }
 
 static bool canEmitDelegateCallArgs(CodeGenFunction &CGF,
@@ -2064,14 +2064,14 @@
  bool Delegating,
  Address This,
  CallArgList &Args,
- AggValueSlot::Overlap_t Overlap) {
+ AggValueSlot::Overlap_t Overlap,
+ SourceLocation Loc) {
   const CXXRecordDecl *ClassDecl = D->getParent();
 
   // C++11 [class.mfct.non-static]p2:
   //   If a non-static member function of a class X is called for an object that
   //   is not of type X, or of a type derived from X, the behavior is undefined.
-  // FIXME: Provide a source location here.
-  EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, SourceLocation(),
+  EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, Loc,
 This.getPointer(), getContext().getRecordType(ClassDecl));
 
   if (D->isTrivial() && D->isDefaultConstructor()) {
@@ -2180,7 +2180,8 @@
   }
 
   EmitCXXConstructorCall(D, Ctor_Base, ForVirtualBase, /*Delegating*/false,
- This, Args, AggValueSlot::MayOverlap);
+ This, Args, AggValueSlot::MayOverlap,
+ E->getLocation());
 }
 
 void CodeGenFunction::EmitInlinedInheritingCXXConstructorCall(
@@ -2277,7 +2278,7 @@
/*ParamsToSkip*/ 1);
 
   EmitCXXConstructorCall(D, Ctor_Complete, false, false, This, Args,
- AggValueSlot::MayOverlap);
+ AggValueSlot::MayOverlap, E->getExprLoc());
 }
 
 void
@@ -2313,7 +2314,7 @@
 
   EmitCXXConstructorCall(Ctor, CtorType, /*ForVirtualBase=*/false,
  /*Delegating=*/true, This, DelegateArgs,
- AggValueSlot::MayOverlap);
+ AggValueSlot::MayOverlap, Loc);
 }
 
 namespace {
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -2362,7 +2362,8 @@
   void EmitCXXConstructorCall(const CXXConstructorDecl *D, CXXCtorType Type,
   bool ForVirtualBase, bool Delegating,
   Address This, CallArgList &Args,
-  AggValueSlot::Overlap_t Overlap);
+  AggValueSlot::Overlap_t Overlap,
+  SourceLocation Loc);
 
   /// Emit assumption load for all bases. Requires to be be called only on
   /// most-derived class and not under construction of the object.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: ioeric, bkramer, hans, thakis, rsmith, zturner.
Herald added a project: clang.

This patch partially reverts D46942 .

If the source file path contains directory junctions, and we resolve them when 
printing
diagnostic messages, these paths look independent for an IDE. For example, both 
Visual
Studio and Visual Studio Code open separate editors for such paths, which is 
not only
inconvenient but might even result in loss of the changes made in one of them.


Repository:
  rC Clang

https://reviews.llvm.org/D59415

Files:
  lib/Basic/FileManager.cpp
  test/Frontend/absolute-paths-windows.test
  test/Frontend/lit.local.cfg


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths 
%t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -497,8 +497,22 @@
   StringRef CanonicalName(Dir->getName());
 
   SmallString<4096> CanonicalNameBuf;
+#ifdef _WIN32
+  CanonicalNameBuf = CanonicalName;
+  llvm::sys::fs::make_absolute(CanonicalNameBuf);
+  llvm::sys::path::native(CanonicalNameBuf);
+  // We've run into needing to remove '..' here in the wild though, so
+  // remove it.
+  // On Windows, symlinks are significantly less prevalent, so removing
+  // '..' is pretty safe.
+  // Ideally we'd have an equivalent of `realpath` and could implement
+  // sys::fs::canonical across all the platforms.
+  llvm::sys::path::remove_dots(CanonicalNameBuf, /* remove_dot_dot */ true);
+  CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#else
   if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
 CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#endif
 
   CanonicalDirNames.insert({Dir, CanonicalName});
   return CanonicalName;


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths %t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -497,8 +497,22 @@
   StringRef CanonicalName(Dir->getName());
 
   SmallString<4096> CanonicalNameBuf;
+#ifdef _WIN32
+  CanonicalNameBuf = CanonicalName;
+  llvm::sys::fs::make_absolute(CanonicalNameBuf);
+  llvm::sys::path::native(CanonicalNameBuf);
+  // We've run into needing to remove '..' here in the wild though, so
+  // remove it.
+  // On Windows, symlinks are significantly less prevalent, so removing
+  // '..' is pretty safe.
+  // Ideally we'd have an equivalent of `realpath` and could implement
+  // sys::fs::canonical across all the platforms.
+  llvm::sys::path::remove_dots(CanonicalNameBuf, /* remove_dot_dot */ true);
+  CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#else
   if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
 CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#endif
 
   CanonicalDirNames.insert({Dir, CanonicalName});
   return CanonicalName;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin marked an inline comment as done.
ikudrin added inline comments.



Comment at: test/Frontend/absolute-paths-windows.test:4
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp

hans wrote:
> This requires (a recent version of) Win 10 or later to run without running as 
> Admin, right?
As far as I know, creating a directory junction, in contrast to a symbolic 
link, does not require escalated privileges. But, honestly, I do not have any 
old system to validate that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin marked an inline comment as done.
ikudrin added inline comments.



Comment at: test/Frontend/absolute-paths-windows.test:4
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp

ikudrin wrote:
> hans wrote:
> > This requires (a recent version of) Win 10 or later to run without running 
> > as Admin, right?
> As far as I know, creating a directory junction, in contrast to a symbolic 
> link, does not require escalated privileges. But, honestly, I do not have any 
> old system to validate that.
We checked that on Windows 7. It does not require rights elevation, too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Ping.

Please note, that, in contrary to POSIX OSes, on Windows a path like 
`dir1\\..` refers to `dir1`. That's why we do not need to ask 
the system for the fully expanded path and can do all calculations internally.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

I am not sure what you mean by "break". Can you share a reproducer?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin updated this revision to Diff 200252.
ikudrin added a comment.

- Made the patch affect only `-fdiagnostics-absolute-paths` option.


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

https://reviews.llvm.org/D59415

Files:
  lib/Frontend/TextDiagnostic.cpp
  test/Frontend/absolute-paths-windows.test
  test/Frontend/lit.local.cfg


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths 
%t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,14 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+#ifdef _WIN32
+  SmallString<4096> DirName = Dir->getName();
+  llvm::sys::fs::make_absolute(DirName);
+  llvm::sys::path::native(DirName);
+  llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+#else
   StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+#endif
   llvm::sys::path::append(AbsoluteFilename, DirName,
   llvm::sys::path::filename(Filename));
   Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths %t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,14 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+#ifdef _WIN32
+  SmallString<4096> DirName = Dir->getName();
+  llvm::sys::fs::make_absolute(DirName);
+  llvm::sys::path::native(DirName);
+  llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+#else
   StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+#endif
   llvm::sys::path::append(AbsoluteFilename, DirName,
   llvm::sys::path::filename(Filename));
   Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin updated this revision to Diff 200749.
ikudrin edited the summary of this revision.
ikudrin added a comment.

Added a comment explaining the differences in the implementations. I would 
really appreciate any corrections.


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

https://reviews.llvm.org/D59415

Files:
  lib/Frontend/TextDiagnostic.cpp
  test/Frontend/absolute-paths-windows.test
  test/Frontend/lit.local.cfg


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths 
%t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,28 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+  // We want to print a simplified absolute path, i. e. without "dots".
+  //
+  // The hardest part here are the paths like "//../".
+  // On Unix-like systems, we cannot just collapse "/..", because
+  // paths are resolved sequentially, and, thereby, the path
+  // "/" may point to a different location. That is why
+  // we use FileManager::getCanonicalName(), which expands all indirections
+  // with llvm::sys::fs::real_path() and caches the result.
+  //
+  // On the other hand, it would be better to preserve as much of the
+  // original path as possible, because that helps a user to recognize it.
+  // real_path() expands all links, which sometimes too much. Luckily,
+  // on Windows we can just use llvm::sys::path::remove_dots(), because,
+  // on that system, both aforementioned paths point to the same place.
+#ifdef _WIN32
+  SmallString<4096> DirName = Dir->getName();
+  llvm::sys::fs::make_absolute(DirName);
+  llvm::sys::path::native(DirName);
+  llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+#else
   StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+#endif
   llvm::sys::path::append(AbsoluteFilename, DirName,
   llvm::sys::path::filename(Filename));
   Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths %t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,28 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+  // We want to print a simplified absolute path, i. e. without "dots".
+  //
+  // The hardest part here are the paths like "//../".
+  // On Unix-like systems, we cannot just collapse "/..", because
+  // paths are resolved sequentially, and, thereby, the path
+  // "/" may point to a different location. That is why
+  // we use FileManager::getCanonicalName(), which expands all indirections
+  // with llvm::sys::fs::real_path() and caches the result.
+  //
+  // On the other hand, it would be better to preserve as much of the
+  // original path as possible, because that helps a user to recognize it.
+  // real_path() expands all links, which sometimes too much. Luckily,
+  // on Windows we can just use l

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361598: Do not resolve directory junctions for 
`-fdiagnostics-absolute-paths` on… (authored by ikudrin, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D59415

Files:
  lib/Frontend/TextDiagnostic.cpp
  test/Frontend/absolute-paths-windows.test
  test/Frontend/lit.local.cfg


Index: test/Frontend/absolute-paths-windows.test
===
--- test/Frontend/absolute-paths-windows.test
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths 
%t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,28 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+  // We want to print a simplified absolute path, i. e. without "dots".
+  //
+  // The hardest part here are the paths like "//../".
+  // On Unix-like systems, we cannot just collapse "/..", because
+  // paths are resolved sequentially, and, thereby, the path
+  // "/" may point to a different location. That is why
+  // we use FileManager::getCanonicalName(), which expands all indirections
+  // with llvm::sys::fs::real_path() and caches the result.
+  //
+  // On the other hand, it would be better to preserve as much of the
+  // original path as possible, because that helps a user to recognize it.
+  // real_path() expands all links, which sometimes too much. Luckily,
+  // on Windows we can just use llvm::sys::path::remove_dots(), because,
+  // on that system, both aforementioned paths point to the same place.
+#ifdef _WIN32
+  SmallString<4096> DirName = Dir->getName();
+  llvm::sys::fs::make_absolute(DirName);
+  llvm::sys::path::native(DirName);
+  llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+#else
   StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+#endif
   llvm::sys::path::append(AbsoluteFilename, DirName,
   llvm::sys::path::filename(Filename));
   Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());


Index: test/Frontend/absolute-paths-windows.test
===
--- test/Frontend/absolute-paths-windows.test
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths %t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,28 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+  // We want to print a simplified absolute path, i. e. without "dots".
+  //
+  // The hardest part here are the paths like "//../".
+  // On Unix-like systems, we cannot just collapse "/..", because
+  // paths are resolved sequentially, and, thereby, the path
+  // "/" may point to a different location. That is why
+  // we use FileManager::getCanonicalName(), which expands all indirections
+  // with llvm::sys::fs::real_path() and caches the result.
+  //
+  // On the other hand, it would be better to preserve as much of the
+  // original path as possible, because that helps a user to recognize it.
+  // real_path() expands all links, 

[PATCH] D121512: [Support] Change zlib::compress to return void

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added inline comments.



Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:568
  DebugCompressionType CompressionType,
  Error &OutErr)
 : SectionBase(Sec), CompressionType(CompressionType),

`OutErr` can now be removed to further simplify the calling code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121512

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

[PATCH] D121512: [Support] Change zlib::compress to return void

[Igor Kudrin via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121512

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

[PATCH] D88021: [clang] Fix a misleading variable name. NFC.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: thakis, hans, MaskRay.
ikudrin added projects: LLVM, clang.
ikudrin requested review of this revision.

The variable is true when frame pointers should be omitted in leaf functions, 
not kept.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88021

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -604,12 +604,12 @@
   bool OmitFP = A && A->getOption().matches(options::OPT_fomit_frame_pointer);
   bool NoOmitFP =
   A && A->getOption().matches(options::OPT_fno_omit_frame_pointer);
-  bool KeepLeaf = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
-   options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS4CPU());
+  bool OmitLeafFP = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
+ options::OPT_mno_omit_leaf_frame_pointer,
+ Triple.isAArch64() || Triple.isPS4CPU());
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
-if (KeepLeaf)
+if (OmitLeafFP)
   return CodeGenOptions::FramePointerKind::NonLeaf;
 return CodeGenOptions::FramePointerKind::All;
   }


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -604,12 +604,12 @@
   bool OmitFP = A && A->getOption().matches(options::OPT_fomit_frame_pointer);
   bool NoOmitFP =
   A && A->getOption().matches(options::OPT_fno_omit_frame_pointer);
-  bool KeepLeaf = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
-   options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS4CPU());
+  bool OmitLeafFP = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
+ options::OPT_mno_omit_leaf_frame_pointer,
+ Triple.isAArch64() || Triple.isPS4CPU());
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
-if (KeepLeaf)
+if (OmitLeafFP)
   return CodeGenOptions::FramePointerKind::NonLeaf;
 return CodeGenOptions::FramePointerKind::All;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88021: [clang] Fix a misleading variable name. NFC.

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG474d527c28f4: [clang] Fix a misleading variable name. NFC. 
(authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88021

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -604,12 +604,12 @@
   bool OmitFP = A && A->getOption().matches(options::OPT_fomit_frame_pointer);
   bool NoOmitFP =
   A && A->getOption().matches(options::OPT_fno_omit_frame_pointer);
-  bool KeepLeaf = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
-   options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS4CPU());
+  bool OmitLeafFP = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
+ options::OPT_mno_omit_leaf_frame_pointer,
+ Triple.isAArch64() || Triple.isPS4CPU());
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
-if (KeepLeaf)
+if (OmitLeafFP)
   return CodeGenOptions::FramePointerKind::NonLeaf;
 return CodeGenOptions::FramePointerKind::All;
   }


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -604,12 +604,12 @@
   bool OmitFP = A && A->getOption().matches(options::OPT_fomit_frame_pointer);
   bool NoOmitFP =
   A && A->getOption().matches(options::OPT_fno_omit_frame_pointer);
-  bool KeepLeaf = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
-   options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS4CPU());
+  bool OmitLeafFP = Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
+ options::OPT_mno_omit_leaf_frame_pointer,
+ Triple.isAArch64() || Triple.isPS4CPU());
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
-if (KeepLeaf)
+if (OmitLeafFP)
   return CodeGenOptions::FramePointerKind::NonLeaf;
 return CodeGenOptions::FramePointerKind::All;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4016
 
+  auto *dwarfArgs =
+  Args.getLastArg(options::OPT_gdwarf64, options::OPT_gdwarf32);

Maybe make this more descriptive?



Comment at: clang/test/Driver/debug-options.c:391
+// GDWARF64_ON:  "-gdwarf64"
+// GDWARF64_OFF:  error: invalid argument '-gdwarf64' only allowed with 
'DWARFv3 or greater'
+// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecture'




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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

[PATCH] D90507: Adding DWARF64 clang flag

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

1. The patch needs tests to check the added functionality.
2. DWARF64 can be generated only for a limited number of targets. There should 
be diagnostics for invalid switch combinations to prevent misuse. @MaskRay 
mentioned that in the patch for `llc`, D87011#2254749 
, but that makes a lot more sense for 
user-level tools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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

[PATCH] D90507: Adding DWARF64 clang flag

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

It looks like `lld/test/COFF/lto-new-pass-manager.ll.obj` was added to the 
patch by accident and should be removed.




Comment at: clang/include/clang/Basic/CodeGenOptions.def:35
 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
+CODEGENOPT(Dwarf64   , 1, 0) ///< -gdwarf64.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.

dblaikie wrote:
> Is there any precedent to draw from for this flag name? (Does GCC support 
> DWARF64? Does it support it under this flag name or some other? (similarly 
> with other gcc-like compilers (Intel's? Whoever else... )))
It looks like we are pioneering in that area. To me, the proposed name looks 
consonant with other debug-related switches.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4025
+  << A->getAsString(Args) << "64 bit architecutre";
+else
+  CmdArgs.push_back("-gdwarf64");

We also should check that the output format is ELF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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

[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2145-2146
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 

ayermolo wrote:
> dblaikie wrote:
> > ayermolo wrote:
> > > dblaikie wrote:
> > > > Does this actually enable debug info emission (the -gdwarf-N versions 
> > > > do cause debug info to be emitted, and set the dwarf version - but, say 
> > > > -ggnu-pubnames does not cause debug info to be emitted if it isn't 
> > > > otherwise requested). Experience has shown it's probably better to have 
> > > > flags like this not enable debug info emission - so they can be used 
> > > > orthogonally (eg: if a project is large, it can add -gdwarf64 to its 
> > > > flags permanently, even though maybe only some build modes, etc, enable 
> > > > debug info (with -g))
> > > > 
> > > > If this doesn't enable debug info emission, the phrasing of the help 
> > > > text might be worth changing somewhat (I wonder how we document 
> > > > -ggnu-pubnames, for instance/comparison/inspiration)
> > > This enables DWARF64 if emission is enabled.
> > > 
> > > From DebugInfo it seems like it's only Elf Format:
> > > 
> > > bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
> > >  DwarfVersion >= 3 &&   // DWARF64 was introduced in 
> > > DWARFv3.
> > >  TT.isArch64Bit() &&// DWARF64 requires 64-bit 
> > > relocations.
> > >  TT.isOSBinFormatELF(); // Support only ELF for now.
> > > This enables DWARF64 if emission is enabled.
> > 
> > Then perhaps the Help Text phrasing should distinguish this behavior from 
> > the behavior of -gdwarf-N which not only sets the version, but enables 
> > emission. (perhaps the phrasing of flags that don't enable emission, like 
> > "-ggnu-pubnames" could provide inspiration for phrasing the help text for 
> > this new flag)
> > 
> > > From DebugInfo it seems like it's only Elf Format:
> > 
> > I'm curious as to why it's ELF only, though - could you explain what the 
> > motivation is there?
> > 
> > (random aside: I still worry about this precedent for -g* flags that 
> > sometimes enable debug info and set a feature, and sometimes they only set 
> > the feature and don't enable debug info - it's a bit of a confusing mess 
> > and I personally rather the -fdebug* flags set a feature but don't enable 
> > emission and -g* flags enable emission, but there's a lack of clear 
> > agreement and there's examples on both sides, so not sure there's much to 
> > be done about that here)
> > This enables DWARF64 if emission is enabled.
> > 
> 
> 
> 
> > From DebugInfo it seems like it's only Elf Format:
> 
> I'm curious as to why it's ELF only, though - could you explain what the 
> motivation is there?
> 

Our customers need DWARF64 for ELF and I do not have much experience with other 
platforms, so I cannot provide adequate support there. Some platform-specific 
tools, like `dsymutil`, have to be fixed to work with DWARF64, but I am 
personally a bit reluctant to do that because I cannot check the result in 
practice. If there will be developers who are willing to support DWARF64 on 
these platforms, they will be able to unlock it and use all the experience we 
collect with ELF, but for now, I prefer to focus on one particular platform 
with practical applications.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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

[PATCH] D96597: [DebugInfo] Keep the DWARF64 flag in the module metadata

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: ayermolo, MaskRay, wenlei, dblaikie.
ikudrin added projects: LLVM, clang, debug-info.
Herald added subscribers: dexonsmith, ormris, hiraditya.
ikudrin requested review of this revision.

This allows the option to affect the LTO output. `Module::Max` helps to 
generate debug info for all modules in the same format.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96597

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/dwarf-format.c
  llvm/include/llvm/IR/Module.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/DebugInfo/X86/dwarf64-module-flag.ll

Index: llvm/test/DebugInfo/X86/dwarf64-module-flag.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/dwarf64-module-flag.ll
@@ -0,0 +1,37 @@
+; This checks that the debug info is generated in the 64-bit format if the
+; module has the corresponding flag.
+
+; RUN: llc -mtriple=x86_64 -filetype=obj %s -o %t
+; RUN: llvm-dwarfdump -debug-info -debug-line %t | FileCheck %s
+
+; CHECK:  Compile Unit: {{.*}} format = DWARF64
+; CHECK:  debug_line[
+; CHECK-NEXT: Line table prologue:
+; CHECK-NEXT:   total_length:
+; CHECK-NEXT: format: DWARF64
+
+; IR generated and reduced from:
+; $ cat foo.c
+; int foo;
+; $ clang -g -gdwarf64 -S -emit-llvm foo.c -o foo.ll
+
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "foo", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 12.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "foo.c", directory: "/tmp")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 7, !"DWARF64", i32 1}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{!"clang version 13.0.0"}
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -509,6 +509,11 @@
   return cast(Val->getValue())->getZExtValue();
 }
 
+bool Module::isDwarf64() const {
+  auto *Val = cast_or_null(getModuleFlag("DWARF64"));
+  return Val && cast(Val->getValue())->isOne();
+}
+
 unsigned Module::getCodeViewFlag() const {
   auto *Val = cast_or_null(getModuleFlag("CodeView"));
   if (!Val)
Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -392,10 +392,11 @@
   DwarfVersion =
   TT.isNVPTX() ? 2 : (DwarfVersion ? DwarfVersion : dwarf::DWARF_VERSION);
 
-  bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
- DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
- TT.isArch64Bit() &&// DWARF64 requires 64-bit relocations.
- TT.isOSBinFormatELF(); // Support only ELF for now.
+  bool Dwarf64 =
+  (Asm->TM.Options.MCOptions.Dwarf64 || MMI->getModule()->isDwarf64()) &&
+  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
+  TT.isArch64Bit() &&// DWARF64 requires 64-bit relocations.
+  TT.isOSBinFormatELF(); // Support only ELF for now.
 
   UseRangesSection = !NoDwarfRangesSection && !TT.isNVPTX();
 
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -805,6 +805,9 @@
   /// Returns the Dwarf Version by checking module flags.
   unsigned getDwarfVersion() const;
 
+  /// Returns the DWARF format by checking module flags.
+  bool isDwarf64() const;
+
   /// Returns the CodeView Version by checking module flags.
   /// Returns zero if not present in module.
   unsigned getCodeViewFlag() const;
Index: clang/test/CodeGen/dwarf-format.c
===
--- /dev/null
+++ clang/test/CodeGen/dwarf-format.c
@@ -0,0 +1,13 @@
+// RUN: %clang -target x86_64-linux-gnu -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=NODWARF64
+// RUN: %clang -target x86_64-linux-gnu -g -gdwarf64 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=DWARF64
+// RUN: %clang -target x86_64-linux-gnu -g -gdwarf64 -gdwarf32 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=NODWARF64
+
+// DWARF64: !{i32 7, !"DWARF64", i32 1}
+// NODWARF64-NOT: !"DWARF64"
+
+int main (void) {
+ 

[PATCH] D96597: [DebugInfo] Keep the DWARF64 flag in the module metadata

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Thank you, @dblaikie!




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:396
+  bool Dwarf64 =
+  (Asm->TM.Options.MCOptions.Dwarf64 || MMI->getModule()->isDwarf64()) &&
+  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.

dblaikie wrote:
> Once this patch is accepted, it's probably worth going back and removing the 
> MCOption? (I don't think we have MCOptions for other debug related module 
> metadata, like the DWARF version?)
Exactly for the DWARF version, there is a similar option, please take a look at 
line 388.
As for the DWARF64 option here, it only simplifies testing with tools like 
`llc`, `opt`, etc. Originally, `MCTargetOptions::Dwarf64` was aimed to adjust 
generating in the assembler, but the corresponding command-line flag is visible 
in other tools, too. Do you think it is worth it to move the command line 
option from the common library code to specific tools?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96597

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

[PATCH] D96783: [Driver] Support -gdwarf64 for assembly files

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: ayermolo, dblaikie, aprantl, MaskRay.
ikudrin added projects: LLVM, clang, debug-info.
Herald added subscribers: jansvoboda11, dang.
ikudrin requested review of this revision.

The option was added in D90507  for C/C++ 
source files. This patch adds support for assembly files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96783

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options-as.c
  clang/test/Misc/cc1as-debug-format.s
  clang/tools/driver/cc1as_main.cpp

Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -91,6 +91,7 @@
   unsigned SaveTemporaryLabels : 1;
   unsigned GenDwarfForAssembly : 1;
   unsigned RelaxELFRelocations : 1;
+  unsigned Dwarf64 : 1;
   unsigned DwarfVersion;
   std::string DwarfDebugFlags;
   std::string DwarfDebugProducer;
@@ -160,6 +161,7 @@
 FatalWarnings = 0;
 NoWarn = 0;
 IncrementalLinkerCompatible = 0;
+Dwarf64 = 0;
 DwarfVersion = 0;
 EmbedBitcode = 0;
   }
@@ -231,6 +233,8 @@
   }
 
   Opts.RelaxELFRelocations = Args.hasArg(OPT_mrelax_relocations);
+  if (auto *DwarfFormatArg = Args.getLastArg(OPT_gdwarf64, OPT_gdwarf32))
+Opts.Dwarf64 = DwarfFormatArg->getOption().matches(OPT_gdwarf64) ? 1 : 0;
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 2, Diags);
   Opts.DwarfDebugFlags =
   std::string(Args.getLastArgValue(OPT_dwarf_debug_flags));
@@ -417,6 +421,7 @@
   Ctx.addDebugPrefixMapEntry(KV.first, KV.second);
   if (!Opts.MainFileName.empty())
 Ctx.setMainFileName(StringRef(Opts.MainFileName));
+  Ctx.setDwarfFormat(Opts.Dwarf64 ? dwarf::DWARF64 : dwarf::DWARF32);
   Ctx.setDwarfVersion(Opts.DwarfVersion);
   if (Opts.GenDwarfForAssembly)
 Ctx.setGenDwarfRootFile(Opts.InputFile,
Index: clang/test/Misc/cc1as-debug-format.s
===
--- /dev/null
+++ clang/test/Misc/cc1as-debug-format.s
@@ -0,0 +1,24 @@
+// REQUIRES: x86-registered-target
+/// DWARF32 debug info is produced by default, when neither -gdwarf32 nor -gdwarf64 is given.
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -debug-info-kind=limited -dwarf-version=4 %s -o %t
+// RUN: llvm-dwarfdump -all %t | FileCheck %s --check-prefixes=CHECK,DWARF32
+/// -gdwarf64 causes generating DWARF64 debug info.
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -gdwarf64 -debug-info-kind=limited -dwarf-version=4 %s -o %t
+// RUN: llvm-dwarfdump -all %t | FileCheck %s --check-prefixes=CHECK,DWARF64
+/// -gdwarf32 is also handled and produces DWARF32 debug info.
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -gdwarf32 -debug-info-kind=limited -dwarf-version=4 %s -o %t
+// RUN: llvm-dwarfdump -all %t | FileCheck %s --check-prefixes=CHECK,DWARF32
+
+// CHECK:.debug_info contents:
+// DWARF32-NEXT: format = DWARF32
+// DWARF64-NEXT: format = DWARF64
+
+// CHECK:.debug_line contents:
+// CHECK-NEXT:   debug_line[
+// CHECK-NEXT:   Line table prologue:
+// CHECK-NEXT: total_length:
+// DWARF32-NEXT: format: DWARF32
+// DWARF64-NEXT: format: DWARF64
+
+.text
+  nop
Index: clang/test/Driver/debug-options-as.c
===
--- clang/test/Driver/debug-options-as.c
+++ clang/test/Driver/debug-options-as.c
@@ -32,3 +32,31 @@
 //
 // P: "-cc1as"
 // P: "-dwarf-debug-producer"
+
+// Check that -gdwarf64 is passed to cc1as.
+// RUN: %clang -### -c -gdwarf64 -gdwarf-5 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-4 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-3 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// GDWARF64_ON: "-cc1as"
+// GDWARF64_ON: "-gdwarf64"
+
+// Check that -gdwarf64 can be reverted with -gdwarf32.
+// RUN: %clang -### -c -gdwarf64 -gdwarf32 -gdwarf-4 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_OFF %s
+// GDWARF64_OFF: "-cc1as"
+// GDWARF64_OFF-NOT: "-gdwarf64"
+
+// Check that an error is reported if -gdwarf64 cannot be used.
+// RUN: %clang -### -c -gdwarf64 -gdwarf-2 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_VER %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-4 -target i386-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_32ARCH %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-4 -target x86_64-apple-darwin %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ELF %s
+//
+// 

[PATCH] D96783: [Driver] Support -gdwarf64 for assembly files

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin marked 2 inline comments as done.
ikudrin added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3757-3771
+  if (DwarfFormatArg &&
+  DwarfFormatArg->getOption().matches(options::OPT_gdwarf64)) {
+if (DwarfVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << DwarfFormatArg->getAsString(Args) << "DWARFv3 or greater";
+else if (!T.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)

dblaikie wrote:
> Could simplify this with an early return:
> ```
> if (!DwarfFormatArg)
>   return;
> ...
> ```
> 
> So DwarfFormatArg isn't tested twice/reduces indentation/etc.
Thanks! I'll update that on commit.



Comment at: clang/tools/driver/cc1as_main.cpp:237
+  if (auto *DwarfFormatArg = Args.getLastArg(OPT_gdwarf64, OPT_gdwarf32))
+Opts.Dwarf64 = DwarfFormatArg->getOption().matches(OPT_gdwarf64) ? 1 : 0;
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 2, Diags);

dblaikie wrote:
> No need for the ` ? 1 : 0`, that's what booleans convert to anyway. (unless 
> there's prior art/other instances of this idiom in this file that this is 
> consistent with)
Thanks! I'll apply the suggestion on commit, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96783

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

[PATCH] D96597: [DebugInfo] Keep the DWARF64 flag in the module metadata

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaa842896299b: [DebugInfo] Keep the DWARF64 flag in the 
module metadata (authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96597

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/dwarf-format.c
  llvm/include/llvm/IR/Module.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/DebugInfo/X86/dwarf64-module-flag.ll

Index: llvm/test/DebugInfo/X86/dwarf64-module-flag.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/dwarf64-module-flag.ll
@@ -0,0 +1,37 @@
+; This checks that the debug info is generated in the 64-bit format if the
+; module has the corresponding flag.
+
+; RUN: llc -mtriple=x86_64 -filetype=obj %s -o %t
+; RUN: llvm-dwarfdump -debug-info -debug-line %t | FileCheck %s
+
+; CHECK:  Compile Unit: {{.*}} format = DWARF64
+; CHECK:  debug_line[
+; CHECK-NEXT: Line table prologue:
+; CHECK-NEXT:   total_length:
+; CHECK-NEXT: format: DWARF64
+
+; IR generated and reduced from:
+; $ cat foo.c
+; int foo;
+; $ clang -g -gdwarf64 -S -emit-llvm foo.c -o foo.ll
+
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "foo", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 12.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "foo.c", directory: "/tmp")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 7, !"DWARF64", i32 1}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{!"clang version 13.0.0"}
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -509,6 +509,11 @@
   return cast(Val->getValue())->getZExtValue();
 }
 
+bool Module::isDwarf64() const {
+  auto *Val = cast_or_null(getModuleFlag("DWARF64"));
+  return Val && cast(Val->getValue())->isOne();
+}
+
 unsigned Module::getCodeViewFlag() const {
   auto *Val = cast_or_null(getModuleFlag("CodeView"));
   if (!Val)
Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -392,10 +392,11 @@
   DwarfVersion =
   TT.isNVPTX() ? 2 : (DwarfVersion ? DwarfVersion : dwarf::DWARF_VERSION);
 
-  bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
- DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
- TT.isArch64Bit() &&// DWARF64 requires 64-bit relocations.
- TT.isOSBinFormatELF(); // Support only ELF for now.
+  bool Dwarf64 =
+  (Asm->TM.Options.MCOptions.Dwarf64 || MMI->getModule()->isDwarf64()) &&
+  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
+  TT.isArch64Bit() &&// DWARF64 requires 64-bit relocations.
+  TT.isOSBinFormatELF(); // Support only ELF for now.
 
   UseRangesSection = !NoDwarfRangesSection && !TT.isNVPTX();
 
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -805,6 +805,9 @@
   /// Returns the Dwarf Version by checking module flags.
   unsigned getDwarfVersion() const;
 
+  /// Returns the DWARF format by checking module flags.
+  bool isDwarf64() const;
+
   /// Returns the CodeView Version by checking module flags.
   /// Returns zero if not present in module.
   unsigned getCodeViewFlag() const;
Index: clang/test/CodeGen/dwarf-format.c
===
--- /dev/null
+++ clang/test/CodeGen/dwarf-format.c
@@ -0,0 +1,13 @@
+// RUN: %clang -target x86_64-linux-gnu -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=NODWARF64
+// RUN: %clang -target x86_64-linux-gnu -g -gdwarf64 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=DWARF64
+// RUN: %clang -target x86_64-linux-gnu -g -gdwarf64 -gdwarf32 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=NODWARF64
+
+// DWARF64: !{i32 7, !"DWARF64", i32 1}
+// NODWARF64-NOT: !"DWARF64"
+
+int main (void) {
+  return 0;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
==

[PATCH] D96783: [Driver] Support -gdwarf64 for assembly files

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ikudrin marked 2 inline comments as done.
Closed by commit rG72eee60b2451: [Driver] Support -gdwarf64 for assembly files 
(authored by ikudrin).

Changed prior to commit:
  https://reviews.llvm.org/D96783?vs=323989&id=324239#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96783

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options-as.c
  clang/test/Misc/cc1as-debug-format.s
  clang/tools/driver/cc1as_main.cpp

Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -91,6 +91,7 @@
   unsigned SaveTemporaryLabels : 1;
   unsigned GenDwarfForAssembly : 1;
   unsigned RelaxELFRelocations : 1;
+  unsigned Dwarf64 : 1;
   unsigned DwarfVersion;
   std::string DwarfDebugFlags;
   std::string DwarfDebugProducer;
@@ -160,6 +161,7 @@
 FatalWarnings = 0;
 NoWarn = 0;
 IncrementalLinkerCompatible = 0;
+Dwarf64 = 0;
 DwarfVersion = 0;
 EmbedBitcode = 0;
   }
@@ -231,6 +233,8 @@
   }
 
   Opts.RelaxELFRelocations = Args.hasArg(OPT_mrelax_relocations);
+  if (auto *DwarfFormatArg = Args.getLastArg(OPT_gdwarf64, OPT_gdwarf32))
+Opts.Dwarf64 = DwarfFormatArg->getOption().matches(OPT_gdwarf64);
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 2, Diags);
   Opts.DwarfDebugFlags =
   std::string(Args.getLastArgValue(OPT_dwarf_debug_flags));
@@ -417,6 +421,7 @@
   Ctx.addDebugPrefixMapEntry(KV.first, KV.second);
   if (!Opts.MainFileName.empty())
 Ctx.setMainFileName(StringRef(Opts.MainFileName));
+  Ctx.setDwarfFormat(Opts.Dwarf64 ? dwarf::DWARF64 : dwarf::DWARF32);
   Ctx.setDwarfVersion(Opts.DwarfVersion);
   if (Opts.GenDwarfForAssembly)
 Ctx.setGenDwarfRootFile(Opts.InputFile,
Index: clang/test/Misc/cc1as-debug-format.s
===
--- /dev/null
+++ clang/test/Misc/cc1as-debug-format.s
@@ -0,0 +1,24 @@
+// REQUIRES: x86-registered-target
+/// DWARF32 debug info is produced by default, when neither -gdwarf32 nor -gdwarf64 is given.
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -debug-info-kind=limited -dwarf-version=4 %s -o %t
+// RUN: llvm-dwarfdump -all %t | FileCheck %s --check-prefixes=CHECK,DWARF32
+/// -gdwarf64 causes generating DWARF64 debug info.
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -gdwarf64 -debug-info-kind=limited -dwarf-version=4 %s -o %t
+// RUN: llvm-dwarfdump -all %t | FileCheck %s --check-prefixes=CHECK,DWARF64
+/// -gdwarf32 is also handled and produces DWARF32 debug info.
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu -filetype obj -gdwarf32 -debug-info-kind=limited -dwarf-version=4 %s -o %t
+// RUN: llvm-dwarfdump -all %t | FileCheck %s --check-prefixes=CHECK,DWARF32
+
+// CHECK:.debug_info contents:
+// DWARF32-NEXT: format = DWARF32
+// DWARF64-NEXT: format = DWARF64
+
+// CHECK:.debug_line contents:
+// CHECK-NEXT:   debug_line[
+// CHECK-NEXT:   Line table prologue:
+// CHECK-NEXT: total_length:
+// DWARF32-NEXT: format: DWARF32
+// DWARF64-NEXT: format: DWARF64
+
+.text
+  nop
Index: clang/test/Driver/debug-options-as.c
===
--- clang/test/Driver/debug-options-as.c
+++ clang/test/Driver/debug-options-as.c
@@ -32,3 +32,31 @@
 //
 // P: "-cc1as"
 // P: "-dwarf-debug-producer"
+
+// Check that -gdwarf64 is passed to cc1as.
+// RUN: %clang -### -c -gdwarf64 -gdwarf-5 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-4 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-3 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// GDWARF64_ON: "-cc1as"
+// GDWARF64_ON: "-gdwarf64"
+
+// Check that -gdwarf64 can be reverted with -gdwarf32.
+// RUN: %clang -### -c -gdwarf64 -gdwarf32 -gdwarf-4 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_OFF %s
+// GDWARF64_OFF: "-cc1as"
+// GDWARF64_OFF-NOT: "-gdwarf64"
+
+// Check that an error is reported if -gdwarf64 cannot be used.
+// RUN: %clang -### -c -gdwarf64 -gdwarf-2 -target x86_64 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_VER %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-4 -target i386-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_32ARCH %s
+// RUN: %clang -### -c -gdwarf64 -gdwarf-4 -target x86_64-apple-darwin %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ELF %s
+//
+// GDWARF64_VER:  

[PATCH] D96865: [Driver] Honor "-gdwarf-N" at any position for assembler sources

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: dblaikie, probinson, aprantl, MaskRay, mehdi_amini.
ikudrin added projects: LLVM, clang, debug-info.
ikudrin requested review of this revision.

This fixes an issue when `-gdwarf-N` switch was ignored if it was given before 
another debug option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96865

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


Index: clang/test/Driver/debug-options-as.c
===
--- clang/test/Driver/debug-options-as.c
+++ clang/test/Driver/debug-options-as.c
@@ -60,3 +60,18 @@
 // GDWARF64_VER:  error: invalid argument '-gdwarf64' only allowed with 
'DWARFv3 or greater'
 // GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecture'
 // GDWARF64_ELF: error: invalid argument '-gdwarf64' only allowed with 'ELF 
platforms'
+
+// Check that -gdwarf-N can be placed before other options of the "-g" group.
+// RUN: %clang -### -c -g -gdwarf-3 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -gdwarf-3 -g -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -g -gdwarf-5 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF5 %s
+// RUN: %clang -### -c -gdwarf-5 -g -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF5 %s
+
+// DWARF3: "-cc1as"
+// DWARF3: "-dwarf-version=3"
+// DWARF5: "-cc1as"
+// DWARF5: "-dwarf-version=5"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -983,6 +983,14 @@
   .Default(0);
 }
 
+// Find a DWARF format version option.
+// This function is a complementary for DwarfVersionNum().
+static const Arg *getDwarfNArg(const ArgList &Args) {
+  return Args.getLastArg(options::OPT_gdwarf_2, options::OPT_gdwarf_3,
+ options::OPT_gdwarf_4, options::OPT_gdwarf_5,
+ options::OPT_gdwarf);
+}
+
 static void RenderDebugEnablingArgs(const ArgList &Args, ArgStringList 
&CmdArgs,
 codegenoptions::DebugInfoKind 
DebugInfoKind,
 unsigned DwarfVersion,
@@ -3850,9 +3858,7 @@
   }
 
   // If a -gdwarf argument appeared, remember it.
-  const Arg *GDwarfN = Args.getLastArg(
-  options::OPT_gdwarf_2, options::OPT_gdwarf_3, options::OPT_gdwarf_4,
-  options::OPT_gdwarf_5, options::OPT_gdwarf);
+  const Arg *GDwarfN = getDwarfNArg(Args);
   bool EmitDwarf = false;
   if (GDwarfN) {
 if (checkDebugInfoOption(GDwarfN, Args, D, TC))
@@ -7175,18 +7181,14 @@
   // Forward -g and handle debug info related flags, assuming we are dealing
   // with an actual assembly file.
   bool WantDebug = false;
-  unsigned DwarfVersion = 0;
   Args.ClaimAllArgs(options::OPT_g_Group);
-  if (Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+  if (Arg *A = Args.getLastArg(options::OPT_g_Group))
 WantDebug = !A->getOption().matches(options::OPT_g0) &&
 !A->getOption().matches(options::OPT_ggdb0);
-if (WantDebug)
-  DwarfVersion = DwarfVersionNum(A->getSpelling());
-  }
 
-  unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(), 
Args);
-  if (DwarfVersion == 0)
-DwarfVersion = DefaultDwarfVersion;
+  unsigned DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+  if (const Arg *GDwarfN = getDwarfNArg(Args))
+DwarfVersion = DwarfVersionNum(GDwarfN->getSpelling());
 
   if (DwarfVersion == 0)
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();


Index: clang/test/Driver/debug-options-as.c
===
--- clang/test/Driver/debug-options-as.c
+++ clang/test/Driver/debug-options-as.c
@@ -60,3 +60,18 @@
 // GDWARF64_VER:  error: invalid argument '-gdwarf64' only allowed with 'DWARFv3 or greater'
 // GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 bit architecture'
 // GDWARF64_ELF: error: invalid argument '-gdwarf64' only allowed with 'ELF platforms'
+
+// Check that -gdwarf-N can be placed before other options of the "-g" group.
+// RUN: %clang -### -c -g -gdwarf-3 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -gdwarf-3 -g -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -g -gdwarf-5 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF5 %s
+// RUN: %clang -### -c -gdwarf-5 -g -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF5 %s
+
+// DWARF3: "-cc1as"
+// DWARF3: "-dwarf-version=3"
+// DWARF5: "-cc1as"
+// DWARF5: "-dwarf-version=5"
In

[PATCH] D96865: [Driver] Honor "-gdwarf-N" at any position for assembler sources

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Thanks!




Comment at: clang/test/Driver/debug-options-as.c:65
+// Check that -gdwarf-N can be placed before other options of the "-g" group.
+// RUN: %clang -### -c -g -gdwarf-3 -integrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s

MaskRay wrote:
> Nit: use the more common spelling `-fintegrated-as`
> 
> `-triple %itanium_abi_triple`
> 
> msvc windows triples ideally don't need `-dwarf-version=`, but that can be a 
> separate clean-up.
> Nit: use the more common spelling -fintegrated-as
`-integrated-as` is what other tests in this file use. Is it OK if these lines 
will not follow the common template?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96865

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

[PATCH] D96865: [Driver] Honor "-gdwarf-N" at any position for assembler sources

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0c9ec1f5e70: [Driver] Honor "-gdwarf-N" at any 
position for assembler sources (authored by ikudrin).

Changed prior to commit:
  https://reviews.llvm.org/D96865?vs=324291&id=324502#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96865

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


Index: clang/test/Driver/debug-options-as.c
===
--- clang/test/Driver/debug-options-as.c
+++ clang/test/Driver/debug-options-as.c
@@ -60,3 +60,18 @@
 // GDWARF64_VER:  error: invalid argument '-gdwarf64' only allowed with 
'DWARFv3 or greater'
 // GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecture'
 // GDWARF64_ELF: error: invalid argument '-gdwarf64' only allowed with 'ELF 
platforms'
+
+// Check that -gdwarf-N can be placed before other options of the "-g" group.
+// RUN: %clang -### -c -g -gdwarf-3 -target %itanium_abi_triple 
-fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -gdwarf-3 -g -target %itanium_abi_triple 
-fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -g -gdwarf-5 -target %itanium_abi_triple 
-fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF5 %s
+// RUN: %clang -### -c -gdwarf-5 -g -target %itanium_abi_triple 
-fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF5 %s
+
+// DWARF3: "-cc1as"
+// DWARF3: "-dwarf-version=3"
+// DWARF5: "-cc1as"
+// DWARF5: "-dwarf-version=5"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -981,6 +981,14 @@
   .Default(0);
 }
 
+// Find a DWARF format version option.
+// This function is a complementary for DwarfVersionNum().
+static const Arg *getDwarfNArg(const ArgList &Args) {
+  return Args.getLastArg(options::OPT_gdwarf_2, options::OPT_gdwarf_3,
+ options::OPT_gdwarf_4, options::OPT_gdwarf_5,
+ options::OPT_gdwarf);
+}
+
 static void RenderDebugEnablingArgs(const ArgList &Args, ArgStringList 
&CmdArgs,
 codegenoptions::DebugInfoKind 
DebugInfoKind,
 unsigned DwarfVersion,
@@ -3848,9 +3856,7 @@
   }
 
   // If a -gdwarf argument appeared, remember it.
-  const Arg *GDwarfN = Args.getLastArg(
-  options::OPT_gdwarf_2, options::OPT_gdwarf_3, options::OPT_gdwarf_4,
-  options::OPT_gdwarf_5, options::OPT_gdwarf);
+  const Arg *GDwarfN = getDwarfNArg(Args);
   bool EmitDwarf = false;
   if (GDwarfN) {
 if (checkDebugInfoOption(GDwarfN, Args, D, TC))
@@ -7168,18 +7174,14 @@
   // Forward -g and handle debug info related flags, assuming we are dealing
   // with an actual assembly file.
   bool WantDebug = false;
-  unsigned DwarfVersion = 0;
   Args.ClaimAllArgs(options::OPT_g_Group);
-  if (Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+  if (Arg *A = Args.getLastArg(options::OPT_g_Group))
 WantDebug = !A->getOption().matches(options::OPT_g0) &&
 !A->getOption().matches(options::OPT_ggdb0);
-if (WantDebug)
-  DwarfVersion = DwarfVersionNum(A->getSpelling());
-  }
 
-  unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(), 
Args);
-  if (DwarfVersion == 0)
-DwarfVersion = DefaultDwarfVersion;
+  unsigned DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+  if (const Arg *GDwarfN = getDwarfNArg(Args))
+DwarfVersion = DwarfVersionNum(GDwarfN->getSpelling());
 
   if (DwarfVersion == 0)
 DwarfVersion = getToolChain().GetDefaultDwarfVersion();


Index: clang/test/Driver/debug-options-as.c
===
--- clang/test/Driver/debug-options-as.c
+++ clang/test/Driver/debug-options-as.c
@@ -60,3 +60,18 @@
 // GDWARF64_VER:  error: invalid argument '-gdwarf64' only allowed with 'DWARFv3 or greater'
 // GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 bit architecture'
 // GDWARF64_ELF: error: invalid argument '-gdwarf64' only allowed with 'ELF platforms'
+
+// Check that -gdwarf-N can be placed before other options of the "-g" group.
+// RUN: %clang -### -c -g -gdwarf-3 -target %itanium_abi_triple -fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -gdwarf-3 -g -target %itanium_abi_triple -fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=DWARF3 %s
+// RUN: %clang -### -c -g -gdwarf-5 -target %itanium_abi_triple -fintegrated-as -x assembler %s 2>&1 \
+// RUN:   | FileCheck -chec

[PATCH] D96865: [Driver] Honor "-gdwarf-N" at any position for assembler sources

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

In D96865#2575486 , @gregmiller wrote:

> Do you have any suggestions on why we would start seeing above failures after 
> you commit?  Was passing before.

From my understanding, the areas seem to be perfectly unrelated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96865

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

[PATCH] D113168: [clang-tblgen] Fix non-determinism in generating AttrSubMatchRulesParserStringSwitches.inc

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: arphaman, aaron.ballman, rsmith.
Herald added a subscriber: mgrang.
ikudrin requested review of this revision.
Herald added a project: clang.

`llvm::MapVector`, compared to `std::map`, guarantees the same iteration order 
in different runs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113168

Files:
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -2043,7 +2044,7 @@
   OS << "  return None;\n";
   OS << "}\n\n";
 
-  std::map>
+  llvm::MapVector>
   SubMatchRules;
   for (const auto &Rule : Rules) {
 if (!Rule.isSubRule())


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -2043,7 +2044,7 @@
   OS << "  return None;\n";
   OS << "}\n\n";
 
-  std::map>
+  llvm::MapVector>
   SubMatchRules;
   for (const auto &Rule : Rules) {
 if (!Rule.isSubRule())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113168: [clang-tblgen] Fix non-determinism in generating AttrSubMatchRulesParserStringSwitches.inc

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

We've noticed binary differences in `clang` in several builds on our local 
build bots, and the patch fixed that.

As for generating the documentation, the issue can be seen, for example, in 
Clang-12 
  
and Clang-13 
 
documentation, where the section "Nullable Attributes" is placed differently. 
Maybe it'll be better to sort the categories alphabetically. Anyway, that is a 
similar but still a different issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113168

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

[PATCH] D113477: [clang-tblgen] Fix non-determinism in generating AttributeReference.rst

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: aaron.ballman, arphaman, rsmith.
ikudrin added a project: clang.
Herald added a subscriber: mgrang.
ikudrin requested review of this revision.

As for now, the categories are printed in an arbitrary order which depends on 
the addresses of dynamically allocated objects. The patch sorts them in an 
alphabetical order thus making the output stable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113477

Files:
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4433,7 +4433,13 @@
   // Gather the Documentation lists from each of the attributes, based on the
   // category provided.
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
-  std::map> SplitDocs;
+  struct CategoryLess {
+bool operator()(const Record *L, const Record *R) const {
+  return L->getValueAsString("Name") < R->getValueAsString("Name");
+}
+  };
+  std::map, CategoryLess>
+  SplitDocs;
   for (const auto *A : Attrs) {
 const Record &Attr = *A;
 std::vector Docs = Attr.getValueAsListOfDefs("Documentation");


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4433,7 +4433,13 @@
   // Gather the Documentation lists from each of the attributes, based on the
   // category provided.
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
-  std::map> SplitDocs;
+  struct CategoryLess {
+bool operator()(const Record *L, const Record *R) const {
+  return L->getValueAsString("Name") < R->getValueAsString("Name");
+}
+  };
+  std::map, CategoryLess>
+  SplitDocs;
   for (const auto *A : Attrs) {
 const Record &Attr = *A;
 std::vector Docs = Attr.getValueAsListOfDefs("Documentation");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113168: [clang-tblgen] Fix non-determinism in generating AttrSubMatchRulesParserStringSwitches.inc

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

The compiler has to be deterministic and one way to check that is to recompile 
a relatively large project several times. For `clang`, that large project is 
usually `clang` itself, so it has to be compiled in a stable way.

Anyway, I've prepared a fix for generating the documentation, D113477 
. Please, take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113168

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

[PATCH] D113477: [clang-tblgen] Fix non-determinism in generating AttributeReference.rst

[Igor Kudrin via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b7ea8e62921: [clang-tblgen] Fix non-determinism in 
generating AttributeReference.rst (authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113477

Files:
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4433,7 +4433,13 @@
   // Gather the Documentation lists from each of the attributes, based on the
   // category provided.
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
-  std::map> SplitDocs;
+  struct CategoryLess {
+bool operator()(const Record *L, const Record *R) const {
+  return L->getValueAsString("Name") < R->getValueAsString("Name");
+}
+  };
+  std::map, CategoryLess>
+  SplitDocs;
   for (const auto *A : Attrs) {
 const Record &Attr = *A;
 std::vector Docs = Attr.getValueAsListOfDefs("Documentation");


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4433,7 +4433,13 @@
   // Gather the Documentation lists from each of the attributes, based on the
   // category provided.
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
-  std::map> SplitDocs;
+  struct CategoryLess {
+bool operator()(const Record *L, const Record *R) const {
+  return L->getValueAsString("Name") < R->getValueAsString("Name");
+}
+  };
+  std::map, CategoryLess>
+  SplitDocs;
   for (const auto *A : Attrs) {
 const Record &Attr = *A;
 std::vector Docs = Attr.getValueAsListOfDefs("Documentation");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113168: [clang-tblgen] Fix non-determinism in generating AttrSubMatchRulesParserStringSwitches.inc

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7352f42cdc3c: [clang-tblgen] Fix non-determinism in 
generating… (authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113168

Files:
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -2043,7 +2044,7 @@
   OS << "  return None;\n";
   OS << "}\n\n";
 
-  std::map>
+  llvm::MapVector>
   SubMatchRules;
   for (const auto &Rule : Rules) {
 if (!Rule.isSubRule())


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -2043,7 +2044,7 @@
   OS << "  return None;\n";
   OS << "}\n\n";
 
-  std::map>
+  llvm::MapVector>
   SubMatchRules;
   for (const auto &Rule : Rules) {
 if (!Rule.isSubRule())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70527: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Personally, I would prefer to see the file name and path to be changed as 
little as possible because that would help to recognize the files better. We 
cannot use `remove_dots()` on POSIX OSes to simplify paths, because it may 
return an invalid path; thus we have to use `getRealPath()`. If I understand it 
right, there is no similar problem with the file name itself.

So, which issues this patch is going to solve?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70527



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

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

In D149119#4312518 , @simon_tatham 
wrote:

> Do LLVM's current portability goals include the constraint that you can only 
> build LLVM for a platform it can also target? If not, then there surely still 
> needs to be //some// kind of escape hatch so that you can avoid needing 
> `llvm-nm` to already support the object file format of the host platform.
>
> I suppose you could say that in that unusual situation it's up to you to 
> adapt `extract_symbols.py` so that it has some other way to get the answers.

If I understand it right, the main targets to use `extract_symbols.py` are AIX 
and Windows. For both these platforms `llvm-nm` and `llvm-readobj` can be 
built. Support for more exotic situations can be added relatively easily should 
it be required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149119

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

[PATCH] D149119: [CMake] Use LLVM own tools in extract_symbols.py

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin updated this revision to Diff 520277.
ikudrin retitled this revision from "[CMake] Use llvm-nm to extract symbols for 
staged LTO builds on Windows" to "[CMake] Use LLVM own tools in 
extract_symbols.py".
ikudrin edited the summary of this revision.
ikudrin added reviewers: chandlerc, beanz, rnk, stevewan, 
hubert.reinterpretcast, DavidSpickett.
ikudrin added a comment.
Herald added a subscriber: MaskRay.
Herald added a reviewer: jhenderson.

- Reworked the patch to always use `llvm-nm` to extract symbols and 
`llvm-readobj` to detect targeting 32-bit Windows.


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

https://reviews.llvm.org/D149119

Files:
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CrossCompile.cmake
  llvm/tools/llvm-nm/CMakeLists.txt
  llvm/tools/llvm-readobj/CMakeLists.txt
  llvm/tools/llvm-shlib/CMakeLists.txt
  llvm/utils/extract_symbols.py

Index: llvm/utils/extract_symbols.py
===
--- llvm/utils/extract_symbols.py
+++ llvm/utils/extract_symbols.py
@@ -23,30 +23,20 @@
 import multiprocessing
 import argparse
 
-# Define functions which extract a list of pairs of (symbols, is_def) from a
-# library using several different tools. We use subprocess.Popen and yield a
-# symbol at a time instead of using subprocess.check_output and returning a list
-# as, especially on Windows, waiting for the entire output to be ready can take
-# a significant amount of time.
-
-def dumpbin_get_symbols(lib):
-process = subprocess.Popen(['dumpbin','/symbols',lib], bufsize=1,
-   stdout=subprocess.PIPE, stdin=subprocess.PIPE,
-   universal_newlines=True)
-process.stdin.close()
-for line in process.stdout:
-# Look for external symbols
-match = re.match("^.+(SECT|UNDEF).+External\s+\|\s+(\S+).*$", line)
-if match:
-yield (match.group(2), match.group(1) != "UNDEF")
-process.wait()
-
-def nm_get_symbols(lib):
-# -P means the output is in portable format, and -g means we only get global
-# symbols.
-cmd = ['nm','-P','-g']
-if sys.platform.startswith('aix'):
-cmd += ['-Xany','-C','-p']
+# Define a function which extracts a list of pairs of (symbols, is_def) from a
+# library using llvm-nm becuase it can work both with regular and bitcode files.
+# We use subprocess.Popen and yield a symbol at a time instead of using
+# subprocess.check_output and returning a list as, especially on Windows, waiting
+# for the entire output to be ready can take a significant amount of time.
+def nm_get_symbols(tool, lib):
+# '-P' means the output is in portable format,
+# '-g' means we only get global symbols,
+# '-Xany' enforce handling both 32- and 64-bit objects on AIX,
+# '--no-demangle' ensure that C++ symbol names are not demangled; note
+#   that llvm-nm do not demangle by default, but the system nm on AIX does
+#   that, so the behavior may change in the future,
+# '-p' do not waste time sorting the symbols.
+cmd = [tool,'-P','-g','-Xany','--no-demangle','-p']
 process = subprocess.Popen(cmd+[lib], bufsize=1,
stdout=subprocess.PIPE, stdin=subprocess.PIPE,
universal_newlines=True)
@@ -68,61 +58,10 @@
 yield (match.group(1), False)
 process.wait()
 
-def readobj_get_symbols(lib):
-process = subprocess.Popen(['llvm-readobj','--symbols',lib], bufsize=1,
-   stdout=subprocess.PIPE, stdin=subprocess.PIPE,
-   universal_newlines=True)
-process.stdin.close()
-for line in process.stdout:
-# When looking through the output of llvm-readobj we expect to see Name,
-# Section, then StorageClass, so record Name and Section when we see
-# them and decide if this is an external symbol when we see
-# StorageClass.
-match = re.search('Name: (\S+)', line)
-if match:
-name = match.group(1)
-match = re.search('Section: (\S+)', line)
-if match:
-section = match.group(1)
-match = re.search('StorageClass: (\S+)', line)
-if match:
-storageclass = match.group(1)
-if section != 'IMAGE_SYM_ABSOLUTE' and \
-   storageclass == 'External':
-yield (name, section != 'IMAGE_SYM_UNDEFINED')
-process.wait()
-
-# Define functions which determine if the target is 32-bit Windows (as that's
+# Define a function which determines if the target is 32-bit Windows (as that's
 # where calling convention name decoration happens).
-
-def dumpbin_is_32bit_windows(lib):
-# dumpbin /headers can output a huge amount of data (>100MB in a debug
-# build) so we read only up to the 'machine' line then close the output.
-process = subprocess.Popen(['dumpbin','/headers',lib], bufsize=1,

[PATCH] D149119: [CMake] Use LLVM own tools in extract_symbols.py

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

In D149119#4329274 , @tmatheson wrote:

> LGTM, thank you for doing this. Please give it a couple of days in case 
> others have comments.

Thanks!

In D149119#4329285 , @jhenderson 
wrote:

> I've not really looked into this patch significantly, so this may well be 
> addressed in the patch, given I see you have modified stuff to do with the 
> NATIVE build, but in the past I have seen LLVM using its own tools to build 
> other parts of its system. I believe it was using llvm-nm to extract the list 
> of symbols needed for export, possibly to do with part of the clang build, 
> possibly even using this script, I don't remember. The problem was that it 
> was using the just-built version of llvm-nm, rather than specifically one 
> from a release build. On a debug build this caused particularly slow builds 
> for me, so much so that I stopped building the relevant parts of LLVM. Please 
> don't introduce a similar situation/make the situation worse (it's quite 
> possible this was fixed some time ago, but I haven't tried recently, nor do I 
> remember the exact thing causing the issue): much like tablegen, any parts of 
> the LLVM build that use just-built tools should make use of release builds, 
> even in debug configuration, at least if an appropriate cmake option is 
> specified.

Your concerns are legit, but the tools in this patch follow the same principle 
as `TableGen`, i.e. if `LLVM_OPTIMIZED_TABLEGEN` is `ON` then the tools are 
forced to be built with optimization.

In D149119#4329618 , @beanz wrote:

> One potential area of concern here: If `llvm-driver` is ever extended to work 
> as a plugin loader (thus exporting its symbols), removing support for the 
> pre-installed host tools could cause a cyclic dependency.

In that case, we will need to add an option to build the tools without plugin 
support so that they can be used in the build process.


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

https://reviews.llvm.org/D149119

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

[PATCH] D149119: [CMake] Use LLVM own tools in extract_symbols.py

[Igor Kudrin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf649599ea933: [CMake] Use LLVM own tools in 
extract_symbols.py (authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149119

Files:
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CrossCompile.cmake
  llvm/tools/llvm-nm/CMakeLists.txt
  llvm/tools/llvm-readobj/CMakeLists.txt
  llvm/tools/llvm-shlib/CMakeLists.txt
  llvm/utils/extract_symbols.py

Index: llvm/utils/extract_symbols.py
===
--- llvm/utils/extract_symbols.py
+++ llvm/utils/extract_symbols.py
@@ -23,30 +23,20 @@
 import multiprocessing
 import argparse
 
-# Define functions which extract a list of pairs of (symbols, is_def) from a
-# library using several different tools. We use subprocess.Popen and yield a
-# symbol at a time instead of using subprocess.check_output and returning a list
-# as, especially on Windows, waiting for the entire output to be ready can take
-# a significant amount of time.
-
-def dumpbin_get_symbols(lib):
-process = subprocess.Popen(['dumpbin','/symbols',lib], bufsize=1,
-   stdout=subprocess.PIPE, stdin=subprocess.PIPE,
-   universal_newlines=True)
-process.stdin.close()
-for line in process.stdout:
-# Look for external symbols
-match = re.match("^.+(SECT|UNDEF).+External\s+\|\s+(\S+).*$", line)
-if match:
-yield (match.group(2), match.group(1) != "UNDEF")
-process.wait()
-
-def nm_get_symbols(lib):
-# -P means the output is in portable format, and -g means we only get global
-# symbols.
-cmd = ['nm','-P','-g']
-if sys.platform.startswith('aix'):
-cmd += ['-Xany','-C','-p']
+# Define a function which extracts a list of pairs of (symbols, is_def) from a
+# library using llvm-nm becuase it can work both with regular and bitcode files.
+# We use subprocess.Popen and yield a symbol at a time instead of using
+# subprocess.check_output and returning a list as, especially on Windows, waiting
+# for the entire output to be ready can take a significant amount of time.
+def nm_get_symbols(tool, lib):
+# '-P' means the output is in portable format,
+# '-g' means we only get global symbols,
+# '-Xany' enforce handling both 32- and 64-bit objects on AIX,
+# '--no-demangle' ensure that C++ symbol names are not demangled; note
+#   that llvm-nm do not demangle by default, but the system nm on AIX does
+#   that, so the behavior may change in the future,
+# '-p' do not waste time sorting the symbols.
+cmd = [tool,'-P','-g','-Xany','--no-demangle','-p']
 process = subprocess.Popen(cmd+[lib], bufsize=1,
stdout=subprocess.PIPE, stdin=subprocess.PIPE,
universal_newlines=True)
@@ -68,61 +58,10 @@
 yield (match.group(1), False)
 process.wait()
 
-def readobj_get_symbols(lib):
-process = subprocess.Popen(['llvm-readobj','--symbols',lib], bufsize=1,
-   stdout=subprocess.PIPE, stdin=subprocess.PIPE,
-   universal_newlines=True)
-process.stdin.close()
-for line in process.stdout:
-# When looking through the output of llvm-readobj we expect to see Name,
-# Section, then StorageClass, so record Name and Section when we see
-# them and decide if this is an external symbol when we see
-# StorageClass.
-match = re.search('Name: (\S+)', line)
-if match:
-name = match.group(1)
-match = re.search('Section: (\S+)', line)
-if match:
-section = match.group(1)
-match = re.search('StorageClass: (\S+)', line)
-if match:
-storageclass = match.group(1)
-if section != 'IMAGE_SYM_ABSOLUTE' and \
-   storageclass == 'External':
-yield (name, section != 'IMAGE_SYM_UNDEFINED')
-process.wait()
-
-# Define functions which determine if the target is 32-bit Windows (as that's
+# Define a function which determines if the target is 32-bit Windows (as that's
 # where calling convention name decoration happens).
-
-def dumpbin_is_32bit_windows(lib):
-# dumpbin /headers can output a huge amount of data (>100MB in a debug
-# build) so we read only up to the 'machine' line then close the output.
-process = subprocess.Popen(['dumpbin','/headers',lib], bufsize=1,
-   stdout=subprocess.PIPE, stdin=subprocess.PIPE,
-   universal_newlines=True)
-process.stdin.close()
-retval = False
-for line in process.stdout:
-match = re.match('.+machine \((\S+)\)', line)
-if match:
-retval = (match.group(1) == 'x86')
-break

[PATCH] D148751: [CMake] Add llvm-lib to Clang bootstrap dependency for LTO builds on Windows

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: smeenai, mstorsjo, phosek, beanz.
ikudrin added a project: LLVM.
Herald added subscribers: ekilmer, inglorion.
Herald added a project: All.
ikudrin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Without this dependency, it is possible that `llvm-lib.exe` will not be built, 
in which case `CMake` will try to use `lib.exe` to build libraries, but this 
tool cannot handle bitcode files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148751

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -613,7 +613,10 @@
   # variable, and have LLVM's CMake append the envar to the archiver calls.
   set(LTO_LIBRARY 
-DDARWIN_LTO_LIBRARY=${LLVM_SHLIB_OUTPUT_INTDIR}/libLTO.dylib
 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR})
-elseif(NOT WIN32)
+elseif(WIN32)
+  add_dependencies(clang-bootstrap-deps llvm-lib)
+  set(${CLANG_STAGE}_AR -DCMAKE_AR=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-lib)
+else()
   add_dependencies(clang-bootstrap-deps llvm-ar llvm-ranlib)
   if(NOT BOOTSTRAP_LLVM_ENABLE_LLD AND LLVM_BINUTILS_INCDIR)
 add_dependencies(clang-bootstrap-deps LLVMgold)


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -613,7 +613,10 @@
   # variable, and have LLVM's CMake append the envar to the archiver calls.
   set(LTO_LIBRARY -DDARWIN_LTO_LIBRARY=${LLVM_SHLIB_OUTPUT_INTDIR}/libLTO.dylib
 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR})
-elseif(NOT WIN32)
+elseif(WIN32)
+  add_dependencies(clang-bootstrap-deps llvm-lib)
+  set(${CLANG_STAGE}_AR -DCMAKE_AR=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-lib)
+else()
   add_dependencies(clang-bootstrap-deps llvm-ar llvm-ranlib)
   if(NOT BOOTSTRAP_LLVM_ENABLE_LLD AND LLVM_BINUTILS_INCDIR)
 add_dependencies(clang-bootstrap-deps LLVMgold)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148751: [CMake] Add llvm-lib to Clang bootstrap dependency for LTO builds on Windows

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin updated this revision to Diff 515215.
ikudrin marked an inline comment as done.
ikudrin added a comment.

- `WIN32` -> `MSVC`

Thanks!


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

https://reviews.llvm.org/D148751

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -613,6 +613,9 @@
   # variable, and have LLVM's CMake append the envar to the archiver calls.
   set(LTO_LIBRARY 
-DDARWIN_LTO_LIBRARY=${LLVM_SHLIB_OUTPUT_INTDIR}/libLTO.dylib
 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR})
+elseif(MSVC)
+  add_dependencies(clang-bootstrap-deps llvm-lib)
+  set(${CLANG_STAGE}_AR -DCMAKE_AR=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-lib)
 elseif(NOT WIN32)
   add_dependencies(clang-bootstrap-deps llvm-ar llvm-ranlib)
   if(NOT BOOTSTRAP_LLVM_ENABLE_LLD AND LLVM_BINUTILS_INCDIR)


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -613,6 +613,9 @@
   # variable, and have LLVM's CMake append the envar to the archiver calls.
   set(LTO_LIBRARY -DDARWIN_LTO_LIBRARY=${LLVM_SHLIB_OUTPUT_INTDIR}/libLTO.dylib
 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR})
+elseif(MSVC)
+  add_dependencies(clang-bootstrap-deps llvm-lib)
+  set(${CLANG_STAGE}_AR -DCMAKE_AR=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-lib)
 elseif(NOT WIN32)
   add_dependencies(clang-bootstrap-deps llvm-ar llvm-ranlib)
   if(NOT BOOTSTRAP_LLVM_ENABLE_LLD AND LLVM_BINUTILS_INCDIR)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148751: [CMake] Add llvm-lib to Clang bootstrap dependency for LTO builds on Windows

[Igor Kudrin via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG43c307fd690f: [CMake] Add llvm-lib to Clang bootstrap 
dependency for LTO builds on Windows (authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148751

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -613,6 +613,9 @@
   # variable, and have LLVM's CMake append the envar to the archiver calls.
   set(LTO_LIBRARY 
-DDARWIN_LTO_LIBRARY=${LLVM_SHLIB_OUTPUT_INTDIR}/libLTO.dylib
 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR})
+elseif(MSVC)
+  add_dependencies(clang-bootstrap-deps llvm-lib)
+  set(${CLANG_STAGE}_AR -DCMAKE_AR=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-lib)
 elseif(NOT WIN32)
   add_dependencies(clang-bootstrap-deps llvm-ar llvm-ranlib)
   if(NOT BOOTSTRAP_LLVM_ENABLE_LLD AND LLVM_BINUTILS_INCDIR)


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -613,6 +613,9 @@
   # variable, and have LLVM's CMake append the envar to the archiver calls.
   set(LTO_LIBRARY -DDARWIN_LTO_LIBRARY=${LLVM_SHLIB_OUTPUT_INTDIR}/libLTO.dylib
 -DDYLD_LIBRARY_PATH=${LLVM_LIBRARY_OUTPUT_INTDIR})
+elseif(MSVC)
+  add_dependencies(clang-bootstrap-deps llvm-lib)
+  set(${CLANG_STAGE}_AR -DCMAKE_AR=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-lib)
 elseif(NOT WIN32)
   add_dependencies(clang-bootstrap-deps llvm-ar llvm-ranlib)
   if(NOT BOOTSTRAP_LLVM_ENABLE_LLD AND LLVM_BINUTILS_INCDIR)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: john.brawn, daltenty, jsji, simon_tatham, tmatheson, 
mstorsjo, phosek.
ikudrin added projects: LLVM, clang.
Herald added subscribers: ekilmer, inglorion.
Herald added a project: All.
ikudrin requested review of this revision.

As for now, `extract_symbols.py` uses a predefined set of tools, none of which 
can read bitcode files. The patch makes it possible to override the used tool 
and passes a fresh built `llvm-nm` for that for multi-staged LTO builds. This 
fixes building plugins with LTO builds and subsequently makes 
`clang/test/Frontend/plugin-*` tests pass.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149119

Files:
  clang/CMakeLists.txt
  llvm/utils/extract_symbols.py

Index: llvm/utils/extract_symbols.py
===
--- llvm/utils/extract_symbols.py
+++ llvm/utils/extract_symbols.py
@@ -29,8 +29,8 @@
 # as, especially on Windows, waiting for the entire output to be ready can take
 # a significant amount of time.
 
-def dumpbin_get_symbols(lib):
-process = subprocess.Popen(['dumpbin','/symbols',lib], bufsize=1,
+def dumpbin_get_symbols(tool, lib):
+process = subprocess.Popen([tool,'/symbols',lib], bufsize=1,
stdout=subprocess.PIPE, stdin=subprocess.PIPE,
universal_newlines=True)
 process.stdin.close()
@@ -41,10 +41,10 @@
 yield (match.group(2), match.group(1) != "UNDEF")
 process.wait()
 
-def nm_get_symbols(lib):
+def nm_get_symbols(tool, lib):
 # -P means the output is in portable format, and -g means we only get global
 # symbols.
-cmd = ['nm','-P','-g']
+cmd = [tool,'-P','-g']
 if sys.platform.startswith('aix'):
 cmd += ['-Xany','-C','-p']
 process = subprocess.Popen(cmd+[lib], bufsize=1,
@@ -68,8 +68,8 @@
 yield (match.group(1), False)
 process.wait()
 
-def readobj_get_symbols(lib):
-process = subprocess.Popen(['llvm-readobj','--symbols',lib], bufsize=1,
+def readobj_get_symbols(tool, lib):
+process = subprocess.Popen([tool,'--symbols',lib], bufsize=1,
stdout=subprocess.PIPE, stdin=subprocess.PIPE,
universal_newlines=True)
 process.stdin.close()
@@ -95,10 +95,10 @@
 # Define functions which determine if the target is 32-bit Windows (as that's
 # where calling convention name decoration happens).
 
-def dumpbin_is_32bit_windows(lib):
+def dumpbin_is_32bit_windows(tool, lib):
 # dumpbin /headers can output a huge amount of data (>100MB in a debug
 # build) so we read only up to the 'machine' line then close the output.
-process = subprocess.Popen(['dumpbin','/headers',lib], bufsize=1,
+process = subprocess.Popen([tool,'/headers',lib], bufsize=1,
stdout=subprocess.PIPE, stdin=subprocess.PIPE,
universal_newlines=True)
 process.stdin.close()
@@ -112,8 +112,8 @@
 process.wait()
 return retval
 
-def objdump_is_32bit_windows(lib):
-output = subprocess.check_output(['objdump','-f',lib],
+def objdump_is_32bit_windows(tool, lib):
+output = subprocess.check_output([tool,'-f',lib],
  universal_newlines=True)
 for line in output.splitlines():
 match = re.match('.+file format (\S+)', line)
@@ -121,8 +121,8 @@
 return (match.group(1) == 'pe-i386')
 return False
 
-def readobj_is_32bit_windows(lib):
-output = subprocess.check_output(['llvm-readobj','--file-header',lib],
+def readobj_is_32bit_windows(tool, lib):
+output = subprocess.check_output([tool,'--file-header',lib],
  universal_newlines=True)
 for line in output.splitlines():
 match = re.match('Format: (\S+)', line)
@@ -132,7 +132,7 @@
 
 # On AIX, there isn't an easy way to detect 32-bit windows objects with the system toolchain,
 # so just assume false.
-def aix_is_32bit_windows(lib):
+def aix_is_32bit_windows(tool, lib):
 return False
 
 # MSVC mangles names to ?@. By examining the
@@ -355,10 +355,10 @@
 return components
 
 def extract_symbols(arg):
-get_symbols, should_keep_symbol, calling_convention_decoration, lib = arg
+get_symbols, get_symbols_tool, should_keep_symbol, calling_convention_decoration, lib = arg
 symbol_defs = dict()
 symbol_refs = set()
-for (symbol, is_def) in get_symbols(lib):
+for (symbol, is_def) in get_symbols(get_symbols_tool, lib):
 symbol = should_keep_symbol(symbol, calling_convention_decoration)
 if symbol:
 if is_def:
@@ -392,8 +392,20 @@
 # Not a template
 return None
 
+def parse_arg_override(parser, val):
+tool, _, path = val.partition('=')
+if not tool in known_tools:
+parser.error(f'Unknown tool: {tool}')
+if not path or not os.path.isfile(path):
+parser.err

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

In D149119#4295110 , @tmatheson wrote:

> This looks like a nice addition. Would it make sense to use llvm-nm always, 
> not restricted to bootstrap builds? And would that work on Windows and allow 
> us to simplify this script substantially by using one tool for all platforms?

If I understand it right, we might not be able to build `llvm-nm` in cases like 
cross-platform building, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149119

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