[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

2022-11-08 Thread Tobias Hieta 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 rGaa99b607b5cf: [clang][pdb] Don't include 
-fmessage-length in PDB buildinfo (authored by thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137322

Files:
  clang/test/CodeGen/debug-info-codeview-buildinfo.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -908,6 +908,9 @@
 }
 if (Arg.startswith("-object-file-name") || Arg == MainFilename)
   continue;
+// Skip fmessage-length for reproduciability.
+if (Arg.startswith("-fmessage-length"))
+  continue;
 if (PrintedOneArg)
   OS << " ";
 llvm::sys::printArg(OS, Arg, /*Quote=*/true);
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -8,6 +8,10 @@
 // RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 
/Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
+// -fmessage-length shouldn't be included in the command line since it breaks 
reproducibility
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc -Xclang 
-fmessage-length=100 /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix 
MESSAGELEN
+
 int main(void) { return 42; }
 
 // CHECK:   Types (.debug$T)
@@ -36,3 +40,8 @@
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  0x{{.+}}: ``
 // DISABLE-NEXT:  : ``
+
+// MESSAGELEN:   Types (.debug$T)
+// MESSAGELEN: 
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT: -fmessage-length


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -908,6 +908,9 @@
 }
 if (Arg.startswith("-object-file-name") || Arg == MainFilename)
   continue;
+// Skip fmessage-length for reproduciability.
+if (Arg.startswith("-fmessage-length"))
+  continue;
 if (PrintedOneArg)
   OS << " ";
 llvm::sys::printArg(OS, Arg, /*Quote=*/true);
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -8,6 +8,10 @@
 // RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
+// -fmessage-length shouldn't be included in the command line since it breaks reproducibility
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc -Xclang -fmessage-length=100 /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix MESSAGELEN
+
 int main(void) { return 42; }
 
 // CHECK:   Types (.debug$T)
@@ -36,3 +40,8 @@
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  0x{{.+}}: ``
 // DISABLE-NEXT:  : ``
+
+// MESSAGELEN:   Types (.debug$T)
+// MESSAGELEN: 
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT: -fmessage-length
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

2022-11-08 Thread Tobias Hieta via Phabricator via cfe-commits 
thieta added a comment.

Thanks @hans


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137322

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

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

2022-11-08 Thread Tobias Hieta via Phabricator via cfe-commits 
thieta updated this revision to Diff 473912.
thieta added a comment.

Updated from feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137322

Files:
  clang/test/CodeGen/debug-info-codeview-buildinfo.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -908,6 +908,9 @@
 }
 if (Arg.startswith("-object-file-name") || Arg == MainFilename)
   continue;
+// Skip fmessage-length for reproduciability.
+if (Arg.startswith("-fmessage-length"))
+  continue;
 if (PrintedOneArg)
   OS << " ";
 llvm::sys::printArg(OS, Arg, /*Quote=*/true);
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -8,6 +8,10 @@
 // RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 
/Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
+// -fmessage-length shouldn't be included in the command line since it breaks 
reproducibility
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc -Xclang 
-fmessage-length=100 /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix 
MESSAGELEN
+
 int main(void) { return 42; }
 
 // CHECK:   Types (.debug$T)
@@ -36,3 +40,8 @@
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  0x{{.+}}: ``
 // DISABLE-NEXT:  : ``
+
+// MESSAGELEN:   Types (.debug$T)
+// MESSAGELEN: 
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT: -fmessage-length


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -908,6 +908,9 @@
 }
 if (Arg.startswith("-object-file-name") || Arg == MainFilename)
   continue;
+// Skip fmessage-length for reproduciability.
+if (Arg.startswith("-fmessage-length"))
+  continue;
 if (PrintedOneArg)
   OS << " ";
 llvm::sys::printArg(OS, Arg, /*Quote=*/true);
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -8,6 +8,10 @@
 // RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
+// -fmessage-length shouldn't be included in the command line since it breaks reproducibility
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc -Xclang -fmessage-length=100 /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix MESSAGELEN
+
 int main(void) { return 42; }
 
 // CHECK:   Types (.debug$T)
@@ -36,3 +40,8 @@
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  0x{{.+}}: ``
 // DISABLE-NEXT:  : ``
+
+// MESSAGELEN:   Types (.debug$T)
+// MESSAGELEN: 
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT: -fmessage-length
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

2022-11-07 Thread Hans Wennborg via Phabricator via cfe-commits 
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm with nits




Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:47
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT:  0x{{.+}}: `"{{.+}}-fmessage-length=

could this be just
MESSAGELEN-NOT: -fmessage-length
?



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:911
   continue;
+// Skip fmessage-length for reproduciability
+if (Arg.startswith("-fmessage-length"))

nit: Period at the end (though I know we're not super consistent about this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137322

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

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

2022-11-07 Thread Tobias Hieta via Phabricator via cfe-commits 
thieta added a reviewer: hans.
thieta added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137322

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

[PATCH] D137322: [clang][pdb] Don't include -fmessage-length in PDB buildinfo

2022-11-03 Thread Tobias Hieta via Phabricator via cfe-commits 
thieta created this revision.
thieta added reviewers: aganea, rnk, thakis, aeubanks.
Herald added a subscriber: hiraditya.
Herald added a project: All.
thieta requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

As discussed in https://reviews.llvm.org/D136474 -fmessage-length
creates problems with reproduciability in the PDB files.

This patch just drops that argument when writing the PDB file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137322

Files:
  clang/test/CodeGen/debug-info-codeview-buildinfo.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -908,6 +908,9 @@
 }
 if (Arg.startswith("-object-file-name") || Arg == MainFilename)
   continue;
+// Skip fmessage-length for reproduciability
+if (Arg.startswith("-fmessage-length"))
+  continue;
 if (PrintedOneArg)
   OS << " ";
 llvm::sys::printArg(OS, Arg, /*Quote=*/true);
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -8,6 +8,10 @@
 // RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 
/Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
+// -fmessage length shouldn't be included in the command line since it breaks 
reproducibility
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc -Xclang 
-fmessage-length=100 /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix 
MESSAGELEN
+
 int main(void) { return 42; }
 
 // CHECK:   Types (.debug$T)
@@ -36,3 +40,8 @@
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  0x{{.+}}: ``
 // DISABLE-NEXT:  : ``
+
+// MESSAGELEN:   Types (.debug$T)
+// MESSAGELEN: 
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT:  0x{{.+}}: `"{{.+}}-fmessage-length=


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -908,6 +908,9 @@
 }
 if (Arg.startswith("-object-file-name") || Arg == MainFilename)
   continue;
+// Skip fmessage-length for reproduciability
+if (Arg.startswith("-fmessage-length"))
+  continue;
 if (PrintedOneArg)
   OS << " ";
 llvm::sys::printArg(OS, Arg, /*Quote=*/true);
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -8,6 +8,10 @@
 // RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
+// -fmessage length shouldn't be included in the command line since it breaks reproducibility
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc -Xclang -fmessage-length=100 /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix MESSAGELEN
+
 int main(void) { return 42; }
 
 // CHECK:   Types (.debug$T)
@@ -36,3 +40,8 @@
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  0x{{.+}}: ``
 // DISABLE-NEXT:  : ``
+
+// MESSAGELEN:   Types (.debug$T)
+// MESSAGELEN: 
+// MESSAGELEN: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// MESSAGELEN-NOT:  0x{{.+}}: `"{{.+}}-fmessage-length=
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits