[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

2018-08-13 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

I don't immediately see how the race on n_alive/op_run happens. It seems that 
the modifications in the thread happen before this line, and modifications in 
main happen after this line. How can both of them modify the variables at the 
same time?


https://reviews.llvm.org/D50549



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


[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

2018-08-13 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

BillyONeal wrote:
> BillyONeal wrote:
> > dvyukov wrote:
> > > I don't immediately see how the race on n_alive/op_run happens. It seems 
> > > that the modifications in the thread happen before this line, and 
> > > modifications in main happen after this line. How can both of them modify 
> > > the variables at the same time?
> > The destruction of g here races with the destruction of the DECAY_COPY'd 
> > copy of G used as the parameter of operator(). That is, line 69 creates a 
> > copy of g, passes that to the started thread, the started thread calls 
> > gCopy(). gCopy() doesn't return until the done flag is set, but the 
> > destruction of the object on which op() is being called is not so 
> > synchronized. Most of the other thread tests avoid this problem by joining 
> > with the thread; joining waits for the destruction of the DECAY_COPY'd 
> > parameters, but this does not.
> > 
> > (This is one of the reasons detach() should basically never be called 
> > anywhere)
> > 
> (That is to say, there's nothing to prevent both threads from executing 
> G::!G() on the two different copies of g... making op_run atomic is probably 
> avoidable but I'm being paranoid given that there was already thread unsafety 
> here...)
What is gCopy? I don't see anything named gCopy in this file...

Do we care about completion of destruction? Why? We wait for done to be set, 
and other variables are already updated at that point. Why does it matter that 
"the destruction of the object on which op() is being called is not so 
synchronized."?


https://reviews.llvm.org/D50549



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


[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

2018-08-13 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

BillyONeal wrote:
> BillyONeal wrote:
> > dvyukov wrote:
> > > BillyONeal wrote:
> > > > BillyONeal wrote:
> > > > > dvyukov wrote:
> > > > > > I don't immediately see how the race on n_alive/op_run happens. It 
> > > > > > seems that the modifications in the thread happen before this line, 
> > > > > > and modifications in main happen after this line. How can both of 
> > > > > > them modify the variables at the same time?
> > > > > The destruction of g here races with the destruction of the 
> > > > > DECAY_COPY'd copy of G used as the parameter of operator(). That is, 
> > > > > line 69 creates a copy of g, passes that to the started thread, the 
> > > > > started thread calls gCopy(). gCopy() doesn't return until the done 
> > > > > flag is set, but the destruction of the object on which op() is being 
> > > > > called is not so synchronized. Most of the other thread tests avoid 
> > > > > this problem by joining with the thread; joining waits for the 
> > > > > destruction of the DECAY_COPY'd parameters, but this does not.
> > > > > 
> > > > > (This is one of the reasons detach() should basically never be called 
> > > > > anywhere)
> > > > > 
> > > > (That is to say, there's nothing to prevent both threads from executing 
> > > > G::!G() on the two different copies of g... making op_run atomic is 
> > > > probably avoidable but I'm being paranoid given that there was already 
> > > > thread unsafety here...)
> > > What is gCopy? I don't see anything named gCopy in this file...
> > > 
> > > Do we care about completion of destruction? Why? We wait for done to be 
> > > set, and other variables are already updated at that point. Why does it 
> > > matter that "the destruction of the object on which op() is being called 
> > > is not so synchronized."?
> > Because the destructor does `--n_alive;`
> >What is gCopy? I don't see anything named gCopy in this file...
> 
> The copy is made in the constructor of std::thread. std::thread makes a copy 
> of all the input parameters, gives the copy to the started thread, and then 
> std::invoke is called there.
> 
> >Why does it matter that "the destruction of the object on which op() is 
> >being called is not so synchronized."?
> 
> Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.
But the first dtor runs before "while (!done) {}" line and the second dtor runs 
after "while (!done) {}" line, no?
Or there is third object involved? But then I don't see how joining the  thread 
would help either.


https://reviews.llvm.org/D50549



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


[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

2018-08-13 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

BillyONeal wrote:
> dvyukov wrote:
> > BillyONeal wrote:
> > > BillyONeal wrote:
> > > > dvyukov wrote:
> > > > > BillyONeal wrote:
> > > > > > BillyONeal wrote:
> > > > > > > dvyukov wrote:
> > > > > > > > I don't immediately see how the race on n_alive/op_run happens. 
> > > > > > > > It seems that the modifications in the thread happen before 
> > > > > > > > this line, and modifications in main happen after this line. 
> > > > > > > > How can both of them modify the variables at the same time?
> > > > > > > The destruction of g here races with the destruction of the 
> > > > > > > DECAY_COPY'd copy of G used as the parameter of operator(). That 
> > > > > > > is, line 69 creates a copy of g, passes that to the started 
> > > > > > > thread, the started thread calls gCopy(). gCopy() doesn't return 
> > > > > > > until the done flag is set, but the destruction of the object on 
> > > > > > > which op() is being called is not so synchronized. Most of the 
> > > > > > > other thread tests avoid this problem by joining with the thread; 
> > > > > > > joining waits for the destruction of the DECAY_COPY'd parameters, 
> > > > > > > but this does not.
> > > > > > > 
> > > > > > > (This is one of the reasons detach() should basically never be 
> > > > > > > called anywhere)
> > > > > > > 
> > > > > > (That is to say, there's nothing to prevent both threads from 
> > > > > > executing G::!G() on the two different copies of g... making op_run 
> > > > > > atomic is probably avoidable but I'm being paranoid given that 
> > > > > > there was already thread unsafety here...)
> > > > > What is gCopy? I don't see anything named gCopy in this file...
> > > > > 
> > > > > Do we care about completion of destruction? Why? We wait for done to 
> > > > > be set, and other variables are already updated at that point. Why 
> > > > > does it matter that "the destruction of the object on which op() is 
> > > > > being called is not so synchronized."?
> > > > Because the destructor does `--n_alive;`
> > > >What is gCopy? I don't see anything named gCopy in this file...
> > > 
> > > The copy is made in the constructor of std::thread. std::thread makes a 
> > > copy of all the input parameters, gives the copy to the started thread, 
> > > and then std::invoke is called there.
> > > 
> > > >Why does it matter that "the destruction of the object on which op() is 
> > > >being called is not so synchronized."?
> > > 
> > > Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.
> > But the first dtor runs before "while (!done) {}" line and the second dtor 
> > runs after "while (!done) {}" line, no?
> > Or there is third object involved? But then I don't see how joining the  
> > thread would help either.
> >But the first dtor runs before "while (!done) {}" line
> 
> No, both dtors are run after the while (!done) {} line. The first dtor runs 
> on line 76 (when the local variable g is destroyed), and the second dtor runs 
> after operator() returns in the constructed thread.  The constructed thread 
> is morally doing:
> 
> ```
> void threadproc(G * g) {
> g->operator(); // setting done happens in here
> delete g; // dtor of second copy runs here
> }
> ```
> 
> > I don't see how joining the thread would help either.
> 
> Joining with the thread would wait for the second dtor -- the one after op() 
> returns -- to complete. Of course joining with the thread isn't doable here 
> given that the point is to test thread::detach :)
> No, both dtors are run after the while (!done) {} line.

But how do we get past while (!done) line before the desctructor in the thread 
has finished? The destructor sets done. So after while (!done) line the 
destructor is effectively finished. What am I missing?


https://reviews.llvm.org/D50549



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


[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

2019-06-24 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

I assume you tried to run it on the kernel. Please post the current output 
somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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


[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

2019-06-24 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Re more checks. I guess we can borrow some from the existing checkers:
https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html
https://bottest.wiki.kernel.org/coccicheck
https://lwn.net/Articles/752408/
https://lwn.net/Articles/691882/

They do some checks very well. But if we could do something better (more true 
positives, less false positives), that may be useful.
Also figuring out which of the existing checks in clang-tidy/analyzer are 
relevant/useful for kernel is another direction.
Also making the thread-safety analysis work for kernel may be a big deal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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


[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

2018-12-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

This needs some tests, at least to exercise interceptors code once.




Comment at: lib/esan/esan_interceptors.cpp:90
   } while (false)
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file)
\
+  do { 
\

The idea behind defining a no-op version in sanitizer_common_interceptors.inc 
was exactly that tools that are not interested in it does not need to be 
changed. So please remove this.



Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259
 
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \
+  if (file) {   \

An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and then 
do:

  if (path)
Acquire(thr, pc, File2addr(path))

Just to not multiply entities. But neither approach looks strictly better to 
me, so I am not too strong about it.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157



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


[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

2019-01-02 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added inline comments.



Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259
 
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \
+  if (file) {   \

krytarowski wrote:
> dvyukov wrote:
> > An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and 
> > then do:
> > 
> >   if (path)
> > Acquire(thr, pc, File2addr(path))
> > 
> > Just to not multiply entities. But neither approach looks strictly better 
> > to me, so I am not too strong about it.
> Is `File2addr()` producing any useful result?
> 
> Reusing `COMMON_INTERCEPTOR_FILE_OPEN` looks fine.
> Is File2addr() producing any useful result?

What exactly do you mean?
It produces a degenerate, but useful result. The original idea was that it can 
return different results for different files, depending on io_sync flag value. 
E.g. not synchronizing deletion of one file with open of another file. But this 
gets tricky with links, bind mounts, etc and was never implemented.
But at least if a file is not involved at all, then we need to skip the Acquire.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157



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


[PATCH] D92846: [KernelAddressSanitizer] Fix globals exclusion for indirect aliases

2020-12-09 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added a comment.
This revision is now accepted and ready to land.

The code looks reasonable to me. I see it only affects kernel, so assuming you 
booted kernel, we should be fine.
I can rubber-stamp it, but if you want more meaningful review, please wait for 
Alex or Nick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92846

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


[PATCH] D90130: Add mips64 support in lib/tsan/go/buildgo.sh

2020-10-26 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

This now looks like a patch for some other change :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90130

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


[PATCH] D90130: Add mips64 support in lib/tsan/go/buildgo.sh

2020-10-26 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added a comment.
This revision is now accepted and ready to land.

Looks good to me.
Do you want me to merge this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90130

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


[PATCH] D90130: Add mips64 support in lib/tsan/go/buildgo.sh

2020-10-26 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov closed this revision.
dvyukov added a comment.

Landed in 
https://github.com/llvm/llvm-project/commit/5cad535ccfebf9b41a57cf2788d8de7a765f7f35


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90130

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


[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

2021-08-13 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:524
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;

When is CurFuncDecl nullptr? Maybe we should look at definition in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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


[PATCH] D105629: [TSan] Add SystemZ support

2021-07-08 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Some solid work here.

I only have a question re mmap handling.




Comment at: compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:781
+  // TSAN. Act as if we ran out of memory.
+  internal_munmap(res, sz);
+  errno = errno_ENOMEM;

TSan runtime is supposed to mprotect all ranges that are inaccessible to the 
application. How does it happen that kernel still returns such an address? Do 
we fail to mprotect all ranges on s390? If so should that be fixed instead?
I am concerned that if kernel can return unsupported addresses, it probably can 
returns them very early before exhausting all supported addresses (e.g. always 
return unsupported from the start).
Or is it that we mprotect everything, but kernel still somehow stomps on it? If 
so can this address overlap with, say, tsan shadow? If yes, them the munmap 
will unmap our shadow which is not good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

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


[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-18 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 468562.
dvyukov added a comment.
Herald added subscribers: llvm-commits, Enna1, hiraditya.
Herald added a project: LLVM.

WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -51,6 +51,9 @@
 
 constexpr uint32_t kVersionBase = 1;// occupies lower 16 bits
 constexpr uint32_t kVersionPtrSizeRel = (1u << 16); // offsets are pointer-sized
+constexpr uint32_t kFeatureNone = 0;
+constexpr uint32_t kFeatureAtomics = 1 << 0;
+constexpr uint32_t kFeatureUAR = 1 << 1;
 constexpr int kCtorDtorPriority = 2;
 
 // Pairs of names of initialization callback functions and which section
@@ -67,14 +70,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
 const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
+ "sanmd_covered", kFeatureNone};
 const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+ "sanmd_atomics", kFeatureAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +92,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +110,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +151,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,12 +245,17 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kFeatureUAR;
+  if (PerInstrFeatureMask & kFeatureUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unam

[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-19 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 468815.
dvyukov added a comment.

WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
-const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
-const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+const MetadataInfo MetadataInfo::Covered{
+"__sanitizer_metadata_covered", "sanmd_covered", kSanitizerMetadataNone};
+const MetadataInfo MetadataInfo::Atomics{
+"__sanitizer_metadata_atomics", "sanmd_atomics", kSanitizerMetadataAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +89,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +107,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +148,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,16 +242,21 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerMetadataUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
-  if (Options.Covered || RequiresCovered) {
+  if (Options.Covered || (PerInstrFeatureMask && RequiresCovered)) {
 NumMetadataCovered++;
 const auto *MI = &MetadataInfo::Covered;
 MIS.insert(MI);
@@ -258,10 +270,25 @@
 }
 
 bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS,
- 

[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-19 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 468816.
dvyukov added a comment.

WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
-const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
-const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+const MetadataInfo MetadataInfo::Covered{
+"__sanitizer_metadata_covered", "sanmd_covered", kSanitizerMetadataNone};
+const MetadataInfo MetadataInfo::Atomics{
+"__sanitizer_metadata_atomics", "sanmd_atomics", kSanitizerMetadataAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +89,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +107,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +148,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,16 +242,21 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerMetadataUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
-  if (Options.Covered || RequiresCovered) {
+  if (Options.Covered || (PerInstrFeatureMask && RequiresCovered)) {
 NumMetadataCovered++;
 const auto *MI = &MetadataInfo::Covered;
 MIS.insert(MI);
@@ -258,10 +270,25 @@
 }
 
 bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS,
- 

[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-19 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Marco, does this look reasonable? Any high-level comments?

There are lots of plumbing to add a new flag and integrate a new pass, so you 
can look only at:
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
llvm/lib/CodeGen/SanitizerMetadata.cpp

As far as I understand the stack layout is only known in machine passes via 
MachineFrameInfo,
so I had to add a machine pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-19 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Unrelated, but looking at the metadata format more closely, I think this can 
benefit from LEB128-like varlen encoding.
Function size is small.
Features are very small.
Stack args size is small.
Function start (if we encode it from the end of the previous one) is very small 
as well.
So potentially it can reduce the entry size from 16 bytes to just 4.
I am just thinking if we use 10x more metadata in future, the size can grow 
from percents of binary to tens of percents.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-19 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 468876.
dvyukov added a comment.

take args alignment into account


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
-const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
-const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+const MetadataInfo MetadataInfo::Covered{
+"__sanitizer_metadata_covered", "sanmd_covered", kSanitizerMetadataNone};
+const MetadataInfo MetadataInfo::Atomics{
+"__sanitizer_metadata_atomics", "sanmd_atomics", kSanitizerMetadataAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +89,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +107,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +148,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,16 +242,21 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerMetadataUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
-  if (Options.Covered || RequiresCovered) {
+  if (Options.Covered || (PerInstrFeatureMask && RequiresCovered)) {
 NumMetadataCovered++;
 const auto *MI = &MetadataInfo::Covered;
 MIS.insert(MI);
@@ -258,10 +270,25 @@
 }
 
 bool SanitizerBinaryMetadata::runOn(Instruction

[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-10-17 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov created this revision.
Herald added a subscriber: ormris.
Herald added a project: All.
dvyukov requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -104,6 +104,7 @@
 enum BinaryMetadataFeature {
   BinaryMetadataCovered = 1 << 0,
   BinaryMetadataAtomics = 1 << 1,
+  BinaryMetadataUAR = 1 << 2,
 };
 
 /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
@@ -1129,7 +1130,8 @@
   // flags. Does not depend on any other sanitizers.
   const std::pair BinaryMetadataFlags[] = {
   std::make_pair(BinaryMetadataCovered, "covered"),
-  std::make_pair(BinaryMetadataAtomics, "atomics")};
+  std::make_pair(BinaryMetadataAtomics, "atomics"),
+  std::make_pair(BinaryMetadataAtomics, "uar")};
   for (const auto &F : BinaryMetadataFlags) {
 if (BinaryMetadataFeatures & F.first)
   CmdArgs.push_back(
@@ -1395,6 +1397,7 @@
 int F = llvm::StringSwitch(Value)
 .Case("covered", BinaryMetadataCovered)
 .Case("atomics", BinaryMetadataAtomics)
+.Case("uar", BinaryMetadataUAR)
 .Case("all", ~0)
 .Default(0);
 if (F == 0 && DiagnoseErrors)
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -234,6 +234,7 @@
   SanitizerBinaryMetadataOptions Opts;
   Opts.Covered = CGOpts.SanitizeBinaryMetadataCovered;
   Opts.Atomics = CGOpts.SanitizeBinaryMetadataAtomics;
+  Opts.UAR = CGOpts.SanitizeBinaryMetadataUAR;
   return Opts;
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5542,6 +5542,10 @@
 : Flag<["-"], "fexperimental-sanitize-metadata=atomics">,
   HelpText<"Emit PCs for atomic operations used by binary analysis 
sanitizers">,
   MarshallingInfoFlag>;
+def fexperimental_sanitize_metadata_EQ_uar
+: Flag<["-"], "fexperimental-sanitize-metadata=uar">,
+  HelpText<"Emit PCs for start of functions that are subject for 
use-after-return checking.">,
+  MarshallingInfoFlag>;
 def fpatchable_function_entry_offset_EQ
 : Joined<["-"], "fpatchable-function-entry-offset=">, MetaVarName<"">,
   HelpText<"Generate M NOPs before function entry">,
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -494,7 +494,7 @@
 
   // Check if any one of SanitizeBinaryMetadata* is enabled.
   bool hasSanitizeBinaryMetadata() const {
-return SanitizeBinaryMetadataCovered || SanitizeBinaryMetadataAtomics;
+return SanitizeBinaryMetadataCovered || SanitizeBinaryMetadataAtomics || 
SanitizeBinaryMetadataUAR;
   }
 };
 
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -288,6 +288,8 @@
 CODEGENOPT(SanitizeCoverageTraceStores, 1, 0) ///< Enable tracing of stores.
 CODEGENOPT(SanitizeBinaryMetadataCovered, 1, 0) ///< Emit PCs for covered 
functions.
 CODEGENOPT(SanitizeBinaryMetadataAtomics, 1, 0) ///< Emit PCs for atomic 
operations.
+CODEGENOPT(SanitizeBinaryMetadataUAR, 1, 0) ///< Emit PCs for start of 
functions
+///< that are subject for 
use-after-return checking.
 CODEGENOPT(SanitizeStats , 1, 0) ///< Collect statistics for sanitizers.
 CODEGENOPT(SimplifyLibCalls  , 1, 1) ///< Set when -fbuiltin is enabled.
 CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -104,6 +104,7 @@
 enum BinaryMetadataFeature {
   BinaryMetadataCovered = 1 << 0,
   BinaryMetadataAtomics = 1 << 1,
+  BinaryMetadataUAR = 1 << 2,
 };
 
 /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
@@ -1129,7 +1130,8 @@
   // flags. Does not depend on any other sanitizers.
   const std::pair BinaryMetadataFlags[] = {
   std::make_pair(BinaryMetadataCovered, "covered"),
-  std::make_pair(BinaryMetadataAt

[PATCH] D145214: [TSAN] add support for riscv64

2023-09-28 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added inline comments.
This revision is now accepted and ready to land.



Comment at: compiler-rt/lib/tsan/rtl/CMakeLists.txt:5
 append_list_if(COMPILER_RT_HAS_MSSE4_2_FLAG -msse4.2 TSAN_RTL_CFLAGS)
-append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=530
+append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=656
TSAN_RTL_CFLAGS)

vitalybuka wrote:
> Maybe this one is not needed after b31bd6d8046d01a66aa92993bacb56b115a67fc5
Yes, is this needed? What function does have larger frame?
If we increase the limit, other arches will slowly slinetly degrade too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145214

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


[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-11-28 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 478168.
dvyukov marked 3 inline comments as done.
dvyukov added a comment.

rebase and adressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
  llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp

Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
@@ -0,0 +1,98 @@
+// We run the compiled binary + sizes of stack arguments depend on the arch.
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | tee /dev/stderr | FileCheck %s
+
+#include 
+#include 
+#include 
+
+// CHECK: metadata add version 1
+
+// CHECK: main: features=2 stack_args=0
+int main() {
+}
+
+// CHECK: non_empty_function: features=2 stack_args=0
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: no_stack_args: features=2 stack_args=0
+void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: stack_args: features=2 stack_args=16
+void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: more_stack_args: features=2 stack_args=32
+void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6, long a7, long a8) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: struct_stack_args: features=2 stack_args=144
+struct large { char x[131]; };
+void struct_stack_args(large a) {
+  volatile int x;
+  x = 1;
+}
+
+
+typedef unsigned long uptr;
+
+const char* function_name(uptr pc) {
+#define FN(X) if (pc == reinterpret_cast(X)) return #X
+  FN(main);
+  FN(non_empty_function);
+  FN(no_stack_args);
+  FN(stack_args);
+  FN(more_stack_args);
+  FN(struct_stack_args);
+#undef FN
+  return nullptr;
+}
+
+template
+T consume(const char*& pos, const char* end) {
+  T v = *reinterpret_cast(pos);
+  pos += sizeof(T);
+  assert(pos <= end);
+  return v;
+}
+
+const char* meta_start;
+const char* meta_end;
+
+extern "C" {
+void __sanitizer_metadata_covered_add(uint32_t version, const char* start, const char* end) {
+  printf("metadata add version %u\n", version);
+  for (const char* pos = start; pos < end;) {
+const uptr base = reinterpret_cast(pos);
+const long offset = (version & (1 << 16)) ?
+consume(pos, end) : consume(pos, end);
+const uint32_t size = consume(pos, end);
+const uint32_t features = consume(pos, end);
+uint32_t stack_args = 0;
+if (features & (1 << 1))
+  stack_args = consume(pos, end);
+if (const char* name = function_name(base + offset))
+  printf("%s: features=%x stack_args=%u\n", name, features, stack_args);
+  }
+  meta_start = start;
+  meta_end = end;
+}
+
+void __sanitizer_metadata_covered_del(uint32_t version, const char* start, const char* end) {
+  assert(start == meta_start);
+  assert(end == meta_end);
+  printf("metadata del version %u\n", version);
+}
+// CHECK: metadata del version 1
+}
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.ll', '.cpp']
Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,16 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {

[PATCH] D136078: [RFC] Use-after-return binary metadata.

2022-11-28 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

PTAL

I've named everything consistently as "SanitizeBinaryMetadata",
fixed interning of metadata, and added a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-28 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added inline comments.



Comment at: llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp:79
+  };
+  MD->replaceOperandWith(1, MDNode::get(F.getContext(), NewAuxMDs));
+  return false;

Should this be a new method on MDBuilder?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-29 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 478569.
dvyukov marked 4 inline comments as done.
dvyukov added a comment.

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/common.h
  llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
  llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp

Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
@@ -0,0 +1,61 @@
+// We run the compiled binary + sizes of stack arguments depend on the arch.
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | tee /dev/stderr | FileCheck %s
+
+// CHECK: metadata add version 1
+
+// CHECK: empty: features=0 stack_args=0
+void empty() {
+}
+
+// CHECK: ellipsis: features=0 stack_args=0
+void ellipsis(const char* fmt, ...) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: non_empty_function: features=2 stack_args=0
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: no_stack_args: features=2 stack_args=0
+void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: stack_args: features=2 stack_args=16
+void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: more_stack_args: features=2 stack_args=32
+void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6, long a7, long a8) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: struct_stack_args: features=2 stack_args=144
+struct large { char x[131]; };
+void struct_stack_args(large a) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: metadata del version 1
+
+#define FUNCTIONS \
+  FN(empty); \
+  FN(ellipsis); \
+  FN(non_empty_function); \
+  FN(no_stack_args); \
+  FN(stack_args); \
+  FN(more_stack_args); \
+  FN(struct_stack_args); \
+/**/
+
+#include "common.h"
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.ll', '.cpp']
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
@@ -0,0 +1,79 @@
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t && %t | FileCheck %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered && %t | FileCheck -check-prefix=CHECK-C %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics && %t | FileCheck -check-prefix=CHECK-A %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=uar && %t | FileCheck -check-prefix=CHECK-U %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics && %t | FileCheck -check-prefix=CHECK-CA %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck -check-prefix=CHECK-CU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+
+// CHECK-NOT: metadata add
+// CHECK: main
+// CHECK-NOT: metadata del
+
+// CHECK-C:  empty: features=0
+// CHECK-A-NOT:  empty:
+// CHECK-U-NOT:  empty:
+// CHECK-CA: empty: features=1
+// CHECK-CU: empty: features=0
+// CHECK-AU-NOT: empty:
+// CHECK-CAU:empty: features=1
+void empty() {
+}
+
+// CHECK-C:  normal: features=0
+// CHECK-A:  normal: features=1
+// CHECK-U:  normal: features=2
+// CHECK-CA: normal: features=1
+// CHECK-CU: normal: features=2
+// CHECK-AU: normal: features=3
+// CHECK-CAU:normal: features=3
+void normal() {
+  volatile int x;
+  x

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-29 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov marked an inline comment as done.
dvyukov added a comment.

PTAL




Comment at: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp:254
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerBinaryMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerBinaryMetadataUAR)

melver wrote:
> What if Options.Covered==true?
> 
> Will it still emit some UAR metadata or will it emit something it shouldn't?
> 
> 
> Should the F.isVarArg() check be done above in `if (PerInstrFeatureMask || 
> (Options.UAR && !F.isVarArg())` ? Then you wouldn't need the 
> `PerInstrFeatureMask && RequiresCovered` change below and it could still just 
> check `RequiresCovered` as before.
> What if Options.Covered==true?
> Will it still emit some UAR metadata or will it emit something it shouldn't?

If Options.Covered==true && F.isVarArg(),  we emit covered metadata with 
features=0. This looks WAI.

> Should the F.isVarArg() check be done above in if (PerInstrFeatureMask || 
> (Options.UAR && !F.isVarArg()) ? Then you wouldn't need the 
> PerInstrFeatureMask && RequiresCovered change below and it could still just 
> check RequiresCovered as before.

It's tricky.
I forgot why I structured the code this way, but I added a new test for all 
permutations of covered/atomics/uar and it shows the following breakage with 
your proposed change:

```
  - CHECK-AU: ellipsis: features=1 stack_args=0
  + CHECK-AU: ellipsis: features=3 stack_args=0
```



Comment at: llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp:1
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t && %t | FileCheck %s

This is the new test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-29 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 478609.
dvyukov marked 2 inline comments as done.
dvyukov added a comment.

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/common.h
  llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
  llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp

Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
@@ -0,0 +1,59 @@
+// We run the compiled binary + sizes of stack arguments depend on the arch.
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | tee /dev/stderr | FileCheck %s
+
+// CHECK: metadata add version 1
+
+// CHECK: empty: features=0 stack_args=0
+void empty() {
+}
+
+// CHECK: ellipsis: features=0 stack_args=0
+void ellipsis(const char* fmt, ...) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: non_empty_function: features=2 stack_args=0
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: no_stack_args: features=2 stack_args=0
+void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: stack_args: features=2 stack_args=16
+void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: more_stack_args: features=2 stack_args=32
+void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6, long a7, long a8) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: struct_stack_args: features=2 stack_args=144
+struct large { char x[131]; };
+void struct_stack_args(large a) {
+  volatile int x;
+  x = 1;
+}
+
+#define FUNCTIONS \
+  FN(empty); \
+  FN(ellipsis); \
+  FN(non_empty_function); \
+  FN(no_stack_args); \
+  FN(stack_args); \
+  FN(more_stack_args); \
+  FN(struct_stack_args); \
+/**/
+
+#include "common.h"
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.ll', '.cpp']
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
@@ -0,0 +1,79 @@
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t && %t | FileCheck %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered && %t | FileCheck -check-prefix=CHECK-C %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics && %t | FileCheck -check-prefix=CHECK-A %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=uar && %t | FileCheck -check-prefix=CHECK-U %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics && %t | FileCheck -check-prefix=CHECK-CA %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck -check-prefix=CHECK-CU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+
+// CHECK-NOT: metadata add
+// CHECK: main
+// CHECK-NOT: metadata del
+
+// CHECK-C:  empty: features=0
+// CHECK-A-NOT:  empty:
+// CHECK-U-NOT:  empty:
+// CHECK-CA: empty: features=1
+// CHECK-CU: empty: features=0
+// CHECK-AU-NOT: empty:
+// CHECK-CAU:empty: features=1
+void empty() {
+}
+
+// CHECK-C:  normal: features=0
+// CHECK-A:  normal: features=1
+// CHECK-U:  normal: features=2
+// CHECK-CA: normal: features=1
+// CHECK-CU: normal: features=2
+// CHECK-AU: normal: features=3
+// CHECK-CAU:normal: features=3
+void normal() {
+  volatile int x;
+  x = 0;
+}
+
+// CHECK-C:   with_atomi

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-29 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov marked an inline comment as done.
dvyukov added inline comments.



Comment at: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp:19
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;

melver wrote:
> Is this comment out of place?
If a function is completely empty, it won't be marked with UAR feature metadata.
This is super-primitive approximation of the actual analysis we should do (the 
function has addr-taken/escaped locals/arguments).
So I needed at least somebody for the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-29 Thread Dmitry Vyukov 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 rGa1255dc467f7: Use-after-return sanitizer binary metadata 
(authored by dvyukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/common.h
  llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
  llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp

Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
@@ -0,0 +1,59 @@
+// We run the compiled binary + sizes of stack arguments depend on the arch.
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | tee /dev/stderr | FileCheck %s
+
+// CHECK: metadata add version 1
+
+// CHECK: empty: features=0 stack_args=0
+void empty() {
+}
+
+// CHECK: ellipsis: features=0 stack_args=0
+void ellipsis(const char* fmt, ...) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: non_empty_function: features=2 stack_args=0
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: no_stack_args: features=2 stack_args=0
+void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: stack_args: features=2 stack_args=16
+void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: more_stack_args: features=2 stack_args=32
+void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6, long a7, long a8) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: struct_stack_args: features=2 stack_args=144
+struct large { char x[131]; };
+void struct_stack_args(large a) {
+  volatile int x;
+  x = 1;
+}
+
+#define FUNCTIONS \
+  FN(empty); \
+  FN(ellipsis); \
+  FN(non_empty_function); \
+  FN(no_stack_args); \
+  FN(stack_args); \
+  FN(more_stack_args); \
+  FN(struct_stack_args); \
+/**/
+
+#include "common.h"
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.ll', '.cpp']
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
@@ -0,0 +1,79 @@
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t && %t | FileCheck %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered && %t | FileCheck -check-prefix=CHECK-C %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics && %t | FileCheck -check-prefix=CHECK-A %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=uar && %t | FileCheck -check-prefix=CHECK-U %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics && %t | FileCheck -check-prefix=CHECK-CA %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck -check-prefix=CHECK-CU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+
+// CHECK-NOT: metadata add
+// CHECK: main
+// CHECK-NOT: metadata del
+
+// CHECK-C:  empty: features=0
+// CHECK-A-NOT:  empty:
+// CHECK-U-NOT:  empty:
+// CHECK-CA: empty: features=1
+// CHECK-CU: empty: features=0
+// CHECK-AU-NOT: empty:
+// CHECK-CAU:empty: features=1
+void empty() {
+}
+
+// CHECK-C:  normal: features=0
+// CHECK-A:  normal: features=1
+// CHECK-U:  normal: features=2
+// CHECK-CA: normal: features=1
+// CHECK-CU: normal: features=2
+// CHECK-AU: normal: features=3
+// CHECK-CAU

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 478826.
dvyukov added a comment.

fixed the debug build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/common.h
  llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
  llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp

Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
@@ -0,0 +1,59 @@
+// We run the compiled binary + sizes of stack arguments depend on the arch.
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | tee /dev/stderr | FileCheck %s
+
+// CHECK: metadata add version 1
+
+// CHECK: empty: features=0 stack_args=0
+void empty() {
+}
+
+// CHECK: ellipsis: features=0 stack_args=0
+void ellipsis(const char* fmt, ...) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: non_empty_function: features=2 stack_args=0
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: no_stack_args: features=2 stack_args=0
+void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: stack_args: features=2 stack_args=16
+void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: more_stack_args: features=2 stack_args=32
+void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6, long a7, long a8) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: struct_stack_args: features=2 stack_args=144
+struct large { char x[131]; };
+void struct_stack_args(large a) {
+  volatile int x;
+  x = 1;
+}
+
+#define FUNCTIONS \
+  FN(empty); \
+  FN(ellipsis); \
+  FN(non_empty_function); \
+  FN(no_stack_args); \
+  FN(stack_args); \
+  FN(more_stack_args); \
+  FN(struct_stack_args); \
+/**/
+
+#include "common.h"
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.ll', '.cpp']
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
@@ -0,0 +1,79 @@
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t && %t | FileCheck %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered && %t | FileCheck -check-prefix=CHECK-C %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics && %t | FileCheck -check-prefix=CHECK-A %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=uar && %t | FileCheck -check-prefix=CHECK-U %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics && %t | FileCheck -check-prefix=CHECK-CA %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck -check-prefix=CHECK-CU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+
+// CHECK-NOT: metadata add
+// CHECK: main
+// CHECK-NOT: metadata del
+
+// CHECK-C:  empty: features=0
+// CHECK-A-NOT:  empty:
+// CHECK-U-NOT:  empty:
+// CHECK-CA: empty: features=1
+// CHECK-CU: empty: features=0
+// CHECK-AU-NOT: empty:
+// CHECK-CAU:empty: features=1
+void empty() {
+}
+
+// CHECK-C:  normal: features=0
+// CHECK-A:  normal: features=1
+// CHECK-U:  normal: features=2
+// CHECK-CA: normal: features=1
+// CHECK-CU: normal: features=2
+// CHECK-AU: normal: features=3
+// CHECK-CAU:normal: features=3
+void normal() {
+  volatile int x;
+  x = 0;
+}
+
+// CHECK-C:   with_atomic: features=0
+// CHECK-A:   with_atomi

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov 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 rGe6aea4a5db09: Use-after-return sanitizer binary metadata 
(authored by dvyukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/common.h
  llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
  llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp

Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
@@ -0,0 +1,59 @@
+// We run the compiled binary + sizes of stack arguments depend on the arch.
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | tee /dev/stderr | FileCheck %s
+
+// CHECK: metadata add version 1
+
+// CHECK: empty: features=0 stack_args=0
+void empty() {
+}
+
+// CHECK: ellipsis: features=0 stack_args=0
+void ellipsis(const char* fmt, ...) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: non_empty_function: features=2 stack_args=0
+void non_empty_function() {
+  // Completely empty functions don't get uar metadata.
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: no_stack_args: features=2 stack_args=0
+void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: stack_args: features=2 stack_args=16
+void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: more_stack_args: features=2 stack_args=32
+void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6, long a7, long a8) {
+  volatile int x;
+  x = 1;
+}
+
+// CHECK: struct_stack_args: features=2 stack_args=144
+struct large { char x[131]; };
+void struct_stack_args(large a) {
+  volatile int x;
+  x = 1;
+}
+
+#define FUNCTIONS \
+  FN(empty); \
+  FN(ellipsis); \
+  FN(non_empty_function); \
+  FN(no_stack_args); \
+  FN(stack_args); \
+  FN(more_stack_args); \
+  FN(struct_stack_args); \
+/**/
+
+#include "common.h"
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.ll', '.cpp']
Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
===
--- /dev/null
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
@@ -0,0 +1,79 @@
+// REQUIRES: native && target-x86_64
+// RUN: clang++ %s -o %t && %t | FileCheck %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered && %t | FileCheck -check-prefix=CHECK-C %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics && %t | FileCheck -check-prefix=CHECK-A %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=uar && %t | FileCheck -check-prefix=CHECK-U %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics && %t | FileCheck -check-prefix=CHECK-CA %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck -check-prefix=CHECK-CU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
+// RUN: clang++ %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+
+// CHECK-NOT: metadata add
+// CHECK: main
+// CHECK-NOT: metadata del
+
+// CHECK-C:  empty: features=0
+// CHECK-A-NOT:  empty:
+// CHECK-U-NOT:  empty:
+// CHECK-CA: empty: features=1
+// CHECK-CU: empty: features=0
+// CHECK-AU-NOT: empty:
+// CHECK-CAU:empty: features=1
+void empty() {
+}
+
+// CHECK-C:  normal: features=0
+// CHECK-A:  normal: features=1
+// CHECK-U:  normal: features=2
+// CHECK-CA: normal: features=1
+// CHECK-CU: normal: features=2
+// CHECK-AU: normal: features=3
+// CHECK-CAU

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Kazu, thanks for taking care of the revert.

I've fixed the assert and tested with -DLLVM_ENABLE_ASSERTIONS=On.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

FTR ./llvm/test/CodeGen/AMDGPU/llc-pipeline.ll also failed as it hardcodes all 
passes:
https://lab.llvm.org/buildbot/#/builders/123/builds/14397


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

In D136078#3959643 , @dvyukov wrote:

> FTR ./llvm/test/CodeGen/AMDGPU/llc-pipeline.ll also failed as it hardcodes 
> all passes:
> https://lab.llvm.org/buildbot/#/builders/123/builds/14397

Also these:

  LLVM :: CodeGen/X86/O0-pipeline.ll
  LLVM :: CodeGen/X86/opt-pipeline.ll

need to grep for something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 478887.
dvyukov added a comment.
Herald added subscribers: kosarev, pcwang-thead, frasercrmck, kerbowa, 
luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, 
the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb, jvesely, nemanjai.

moved *.cpp tests to clang/test/
updated tests that hardcode pipeline passes
fixed metadata update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Instrumentation/SanitizerBinaryMetadata/common.h
  clang/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  clang/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/M68k/pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -205,6 +205,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -71,6 +71,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/RISCV/O3-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -163,6 +163,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT: Machine Outliner
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
Index: llvm/test/CodeGen/RISCV/O0-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -56,6 +56,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
 ; CHECK-NEXT:   RISCV atomic pseudo instruction expansion pass
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/PowerPC/O3-pipeline.ll
===
--- llvm/test/CodeGen/PowerPC/O3-pipeline.ll
+++ llvm/test/CodeGen/PowerPC/O3-pipeline.ll
@@ -206,6 +206,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   PowerPC Expand Atomic
 ; CHECK-NEXT:   PowerPC Branch Selector
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/M68k/pipeline.ll
===
--- llvm/test/CodeGen/M68k/pipeline.ll
+++ llvm/test/CodeGen/M68k/pipeline.ll
@@ -

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

PTAL

moved *.cpp tests to clang/test/
updated tests that hardcode pipeline passes
fixed metadata update

This did not work, it also changed all shared instances:

  const auto *NewMD = MDB.createPCSections(
  {{Section.getString(), {Features, IRB.getInt32(Size)}}});
  MD->replaceOperandWith(1, NewMD->getOperand(1));


so I changed it to:

  F.setMetadata(LLVMContext::MD_pcsections,
MDB.createPCSections(
{{Section.getString(), {Features, IRB.getInt32(Size)}}}));


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 478912.
dvyukov added a comment.

restrict tests to linux


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Instrumentation/SanitizerBinaryMetadata/common.h
  clang/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  clang/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/M68k/pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -205,6 +205,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -71,6 +71,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/RISCV/O3-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -163,6 +163,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT: Machine Outliner
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
Index: llvm/test/CodeGen/RISCV/O0-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -56,6 +56,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
 ; CHECK-NEXT:   RISCV atomic pseudo instruction expansion pass
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/PowerPC/O3-pipeline.ll
===
--- llvm/test/CodeGen/PowerPC/O3-pipeline.ll
+++ llvm/test/CodeGen/PowerPC/O3-pipeline.ll
@@ -206,6 +206,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   PowerPC Expand Atomic
 ; CHECK-NEXT:   PowerPC Branch Selector
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/M68k/pipeline.ll
===
--- llvm/test/CodeGen/M68k/pipeline.ll
+++ llvm/test/CodeGen/M68k/pipeline.ll
@@ -129,6 +129,7 @@
 ; CHECK-NEXT:  Contiguously Lay Out Funclets
 ; CHECK-NEXT:  StackMap Liveness Analysis
 ; CHECK-NEXT:  Live DEBUG_VALUE analysis
+; CHECK-NEXT:  Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:  Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:  Machine Optimization Remark Emitter
 ; CHECK-NEXT:  M68k Assembly Printer
I

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-11-30 Thread Dmitry Vyukov 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 rGd3c851d3fc8b: Use-after-return sanitizer binary metadata 
(authored by dvyukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Instrumentation/SanitizerBinaryMetadata/common.h
  clang/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  clang/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/M68k/pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -205,6 +205,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -71,6 +71,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/RISCV/O3-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -163,6 +163,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT: Machine Outliner
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
Index: llvm/test/CodeGen/RISCV/O0-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -56,6 +56,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
 ; CHECK-NEXT:   RISCV atomic pseudo instruction expansion pass
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/PowerPC/O3-pipeline.ll
===
--- llvm/test/CodeGen/PowerPC/O3-pipeline.ll
+++ llvm/test/CodeGen/PowerPC/O3-pipeline.ll
@@ -206,6 +206,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   PowerPC Expand Atomic
 ; CHECK-NEXT:   PowerPC Branch Selector
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/M68k/pipeline.ll
===
--- llvm/test/CodeGen/M68k/pipeline.ll
+++ llvm/test/CodeGen/M68k/pipeline.ll
@@ -129,6 +129,7 @@
 ; CHECK-NEXT:  Contiguously Lay Out Funclets
 ; CHECK-NEXT:  StackMap Liveness Analysis
 ; CHECK-NEXT:  Live DEBUG_VALUE analysis
+; CHECK-NEXT:  Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:  Lazy

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-01 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

@melver Re this failure:

   TEST 'Clang :: 
Instrumentation/SanitizerBinaryMetadata/uar.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 4';   
/home/buildbot/as-worker-91/clang-with-lto-ubuntu/build/stage1/bin/clang 
--driver-mode=g++ 
/home/buildbot/as-worker-91/clang-with-lto-ubuntu/llvm-project/clang/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
 -o 
/home/buildbot/as-worker-91/clang-with-lto-ubuntu/build/stage1/tools/clang/test/Instrumentation/SanitizerBinaryMetadata/Output/uar.cpp.tmp
 -fexperimental-sanitize-metadata=covered,uar && 
/home/buildbot/as-worker-91/clang-with-lto-ubuntu/build/stage1/tools/clang/test/Instrumentation/SanitizerBinaryMetadata/Output/uar.cpp.tmp
 | /home/buildbot/as-worker-91/clang-with-lto-ubuntu/build/stage1/bin/FileCheck 
/home/buildbot/as-worker-91/clang-with-lto-ubuntu/llvm-project/clang/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp
  --
  Exit Code: 1
  Command Output (stderr):
  --
  /usr/local/bin/ld: sanmd_covered has both ordered 
[`sanmd_covered[_Z7consumeIjET_RPKcS2_]' in /tmp/lit-tmp-2jisr97l/uar-d5d8d4.o] 
and unordered [`sanmd_covered[__dummy_sanmd_covered]' in 
/tmp/lit-tmp-2jisr97l/uar-d5d8d4.o] sections
  /usr/local/bin/ld: final link failed: Bad value
  clang-16: error: linker command failed with exit code 1 (use -v to see 
invocation)

https://lab.llvm.org/buildbot/#/builders/124/builds/5759/steps/7/logs/stdio

This looks like a latent issue which is just exposed by the test.

As far as I understand this happens because this dummy variable somehow ends up 
in an "unordered" section, while other "real" metadata objects end up in 
"ordered" sections:

  void SanitizerBinaryMetadata::createZeroSizedObjectInSection(
  Type *Ty, StringRef SectionSuffix) {
auto *DummyInit = ConstantAggregateZero::get(ArrayType::get(Ty, 0));
auto *DummyEntry = new GlobalVariable(Mod, DummyInit->getType(), true,
  GlobalVariable::ExternalLinkage,
  DummyInit, "__dummy_" + 
SectionSuffix);
DummyEntry->setSection(getSectionName(SectionSuffix));
DummyEntry->setVisibility(GlobalValue::HiddenVisibility);
if (TargetTriple.supportsCOMDAT())
  DummyEntry->setComdat(Mod.getOrInsertComdat(DummyEntry->getName()));
// Make sure the section isn't discarded by gc-sections.
appendToUsed(Mod, DummyEntry);
  }

I don't see any method on GlobalVariable to set the section to "ordered".

The only idea I have is to print dummy object in AsmPrinter the same way we 
print all other metadata objects (then I assume it will have the same 
properties as other objects).

Do you have any other ideas on how to dead with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-01 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Re this failure:

  project/clang/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  covered.cpp.tmp: 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Instrumentation/SanitizerBinaryMetadata/common.h:22:
 T consume(const char *&, const char *) [T = unsigned int]: Assertion `pos <= 
end' failed.
  FileCheck error: '' is empty.
  FileCheck command line:  /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
-check-prefix=CHECK-A 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Instrumentation/SanitizerBinaryMetadata/covered.cpp

https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8796062278266465473/+/u/clang/test/stdout

Not sure why it run on fuchsia at all and the strange failure mode.
But I hope this will go away when we move tests to compiler-rt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D143482: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args

2023-02-08 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added a comment.
This revision is now accepted and ready to land.

Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143482

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


[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

2023-02-09 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/fsanitize-metadata-ignorelist.c:6
+// RUN: %clang -target aarch64-linux-gnu -fexperimental-sanitize-metadata=all 
-fexperimental-sanitize-metadata-ignorelist=%t.good 
-fexperimental-sanitize-metadata-ignorelist=%t.second %s -### 2>&1 | FileCheck 
%s
+// CHECK: -fexperimental-sanitize-metadata-ignorelist={{.*}}.good" 
"-fexperimental-sanitize-metadata-ignorelist={{.*}}.second

What is this test doing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143664

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


[PATCH] D138489: [tsan] Add tsan support for loongarch64

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov accepted this revision.
dvyukov added inline comments.



Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp:1526
+
+  /* if ($a0 != 0)
+   *   return $a0;

Please use C-style one-line comments //


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138489

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov updated this revision to Diff 480051.
dvyukov added a comment.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

Moved tests to compiler-rt and rebased to HEAD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/metadata/CMakeLists.txt
  compiler-rt/test/metadata/common.h
  compiler-rt/test/metadata/covered.cpp
  compiler-rt/test/metadata/lit.cfg.py
  compiler-rt/test/metadata/lit.site.cfg.py.in
  compiler-rt/test/metadata/uar.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/M68k/pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -206,6 +206,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -72,6 +72,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/RISCV/O3-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -164,6 +164,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT: Machine Outliner
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
Index: llvm/test/CodeGen/RISCV/O0-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -57,6 +57,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
 ; CHECK-NEXT:   RISCV atomic pseudo instruction expansion pass
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/PowerPC/O3-pipeline.ll
===
--- llvm/test/CodeGen/PowerPC/O3-pipeline.ll
+++ llvm/test/CodeGen/PowerPC/O3-pipeline.ll
@@ -207,6 +207,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   PowerPC Expand Atomic
 ; CHECK-NEXT:   PowerPC Branch Selector
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/M68k/pipeline.ll
===
--- llvm/test/CodeGen/M68k/pipeline.ll
+++ llvm/test/CodeGen/M68k/pipeline.ll
@@ -131,6 +131,7 @@
 ; CHECK-NEXT:  Contiguously Lay Out Funclets
 ; CHECK-NEXT:  StackMap Liveness Analysis
 ; CHECK-NEXT:  Live DEBUG_VALUE analysis
+; CHECK-NEXT:  Mach

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov 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 rGdbe8c2c316c4: Use-after-return sanitizer binary metadata 
(authored by dvyukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/metadata/CMakeLists.txt
  compiler-rt/test/metadata/common.h
  compiler-rt/test/metadata/covered.cpp
  compiler-rt/test/metadata/lit.cfg.py
  compiler-rt/test/metadata/lit.site.cfg.py.in
  compiler-rt/test/metadata/uar.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/M68k/pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -206,6 +206,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -72,6 +72,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   X86 Speculative Execution Side Effect Suppression
 ; CHECK-NEXT:   X86 Indirect Thunks
 ; CHECK-NEXT:   X86 Return Thunks
Index: llvm/test/CodeGen/RISCV/O3-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -164,6 +164,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT: Machine Outliner
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
Index: llvm/test/CodeGen/RISCV/O0-pipeline.ll
===
--- llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -57,6 +57,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   RISCV pseudo instruction expansion pass
 ; CHECK-NEXT:   RISCV atomic pseudo instruction expansion pass
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/PowerPC/O3-pipeline.ll
===
--- llvm/test/CodeGen/PowerPC/O3-pipeline.ll
+++ llvm/test/CodeGen/PowerPC/O3-pipeline.ll
@@ -207,6 +207,7 @@
 ; CHECK-NEXT:   Contiguously Lay Out Funclets
 ; CHECK-NEXT:   StackMap Liveness Analysis
 ; CHECK-NEXT:   Live DEBUG_VALUE analysis
+; CHECK-NEXT:   Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:   PowerPC Expand Atomic
 ; CHECK-NEXT:   PowerPC Branch Selector
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
Index: llvm/test/CodeGen/M68k/pipeline.ll
===
--- llvm/test/CodeGen/M68k/pipeline.ll
+++ llvm/test/CodeGen/M68k/pipeline.ll
@@ -131,6 +131,7 @@
 ; CHECK-NEXT:  Contiguously Lay Out Funclets
 ; CHECK-NEXT:  StackMap Liveness Analysis
 ; CHECK-NEXT:  Live DEBUG_VALUE

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

In D136078#3970792 , @thakis wrote:

> This seems to break tests: http://45.33.8.238/linux/93224/step_12.txt
>
> Can you take a look?

Looking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Fix for:

  -- Supported architectures for crt: aarch64
  CMake Error at compiler-rt/cmake/config-ix.cmake:244 (message):
Unsupported architecture: x86_64
  Call Stack (most recent call first):
compiler-rt/cmake/config-ix.cmake:280 (get_target_flags_for_arch)
compiler-rt/test/metadata/CMakeLists.txt:7 (get_test_cc_for_arch)
  -- Configuring incomplete, errors occurred!

https://reviews.llvm.org/D139323


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

In D136078#3970792 , @thakis wrote:

> This seems to break tests: http://45.33.8.238/linux/93224/step_12.txt
>
> Can you take a look?

Sent a fix: https://reviews.llvm.org/D139325


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

FTR 2 breakage fixes merged:
https://github.com/llvm/llvm-project/commit/2a05bd212e3e8aaed818ee23464f4d1fe0b0596d
https://github.com/llvm/llvm-project/commit/08742e72a34e835e6fc3c696eabe6045c78d6289


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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


[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly

2017-01-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

Yes, for tsan runtime detection does not make lots of sense -- we only have few 
SSE instructions on inlined fast path. The SSE code provides quite substantial 
speedup. User -march flag won't help as runtime is prebuilt. SCUDO is sse4.2, 
but tsan is sse3. Do you actually see any problems with sse3? It's like 10 
years old. I agree with you that strictly speaking we do wrong thing for old 
processors. But if we disable it, we disable it for everybody.  So if you don't 
hit cashes with it, I would prefer to keep sse3 in tsan.


https://reviews.llvm.org/D28304



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


[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly

2017-01-05 Thread Dmitry Vyukov via Phabricator via cfe-commits
dvyukov added a comment.

> I don't see a big issue enabling it for x86-64 but for 32-bit systems you're 
> ruling out quite a lot of hardware.

Tsan does not support 32-bits.


https://reviews.llvm.org/D28304



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