[PATCH] D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests.

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

sanitizer-x86_64-linux https://lab.llvm.org/buildbot/#/builders/37/builds/4244 
FAIL: ScudoStandalone-x86_64 :: preinit.c (772 of 856)
ppc64be-clang-test https://lab.llvm.org/buildbot#builders/52/builds/7794 TEST 
'ScudoStandalone-powerpc64 :: preinit.c' FAILED
among others


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102543

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

[PATCH] D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests.

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

I saw some bots failure for preinit.c:

  FAIL: ScudoStandalone-i386 :: preinit.c (768 of 856)
   TEST 'ScudoStandalone-i386 :: preinit.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';  
/b/sanitizer-x86_64-linux/build/clang_build/./bin/clang   -m32  -pthread -fPIE 
-pie -O0 -UNDEBUG -Wl,--gc-sections 
-resource-dir=/b/sanitizer-x86_64-linux/build/clang_build/./lib/clang/13.0.0/lib/linux/../../
 -fsanitize=scudo 
/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/scudo/standalone/preinit.c
 -o 
/b/sanitizer-x86_64-linux/build/clang_build/projects/compiler-rt/test/scudo/standalone/I386LinuxConfig/Output/preinit.c.tmp
  : 'RUN: at line 2';
/b/sanitizer-x86_64-linux/build/clang_build/projects/compiler-rt/test/scudo/standalone/I386LinuxConfig/Output/preinit.c.tmp
 2>&1
  --
  Exit Code: 139
  
  Command Output (stderr):
  --
  
/b/sanitizer-x86_64-linux/build/clang_build/projects/compiler-rt/test/scudo/standalone/I386LinuxConfig/Output/preinit.c.script:
 line 2: 20628 Segmentation fault  
/b/sanitizer-x86_64-linux/build/clang_build/projects/compiler-rt/test/scudo/standalone/I386LinuxConfig/Output/preinit.c.tmp
 2>&1

Do you know what went on with those?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102543

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

[PATCH] D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests.

[Kostya Kortchinsky via Phabricator via cfe-commits]

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

I think this is good, with the additional arch




Comment at: compiler-rt/cmake/config-ix.cmake:338
 set(ALL_SCUDO_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS32} 
${MIPS64} ${PPC64})
-set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64})
+set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64} ${MIPS32} 
${MIPS64} ${PPC64})
 if(APPLE)

${ARM32} as well please


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102543

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

[PATCH] D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests.

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

I think one of the remaining things to address is the supported architectures:

  set(ALL_SCUDO_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS32} 
${MIPS64} ${PPC64})
  set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64})

I am unclear as to whether or not there are consumers for the missing ones, but 
if there are we might break -fsanitize=scudo for them.
We have fallback for non-x86/arm processors in 
https://github.com/llvm/llvm-project/blob/e0921655b1ff8d4ba7c14be59252fe05b705920e/compiler-rt/lib/scudo/standalone/checksum.cpp#L79
 , the rest looks fine so I think adding them should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102543

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

[PATCH] D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests.

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

Thanks a lot Mitch! Couple of comments from a first look, I'll give it another 
go later.




Comment at: compiler-rt/lib/scudo/standalone/wrappers_cpp.cpp:42
+  if (Allocator.canReturnNull()) { 
\
+errno = EINVAL;
\
+return nullptr;
\

I don't think errno carry over to  C++, there seems to be no sign of it in 
https://en.cppreference.com/w/cpp/memory/new/operator_new



Comment at: compiler-rt/test/scudo/standalone/lit.cfg.py:21
 c_flags = ([config.target_cflags] +
-   ["-pthread",
-   "-fPIE",
-   "-pie",
-   "-O0",
-   "-UNDEBUG",
-   "-ldl",
-   "-Wl,--gc-sections"])
+   ["-pthread", "-fPIE", "-pie", "-O0", "-UNDEBUG", "-ldl",
+"-Wl,--gc-sections"])

-ldl might not be necessary anymore without the sanitizer dependencies?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102543

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

[PATCH] D97496: [Clang][ASan] Correct AsanDtorKindToString to return non-void in default case

[Kostya Kortchinsky 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 rG41751b637317: [Clang][ASan] Correct AsanDtorKindToString to 
return non-void in default case (authored by cryptoad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97496

Files:
  clang/lib/Basic/Sanitizers.cpp


Index: clang/lib/Basic/Sanitizers.cpp
===
--- clang/lib/Basic/Sanitizers.cpp
+++ clang/lib/Basic/Sanitizers.cpp
@@ -70,6 +70,7 @@
   case llvm::AsanDtorKind::Invalid:
 return "invalid";
   }
+  return "invalid";
 }
 
 llvm::AsanDtorKind AsanDtorKindFromString(StringRef kindStr) {


Index: clang/lib/Basic/Sanitizers.cpp
===
--- clang/lib/Basic/Sanitizers.cpp
+++ clang/lib/Basic/Sanitizers.cpp
@@ -70,6 +70,7 @@
   case llvm::AsanDtorKind::Invalid:
 return "invalid";
   }
+  return "invalid";
 }
 
 llvm::AsanDtorKind AsanDtorKindFromString(StringRef kindStr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97496: [Clang][ASan] Correct AsanDtorKindToString to return non-void in default case

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad updated this revision to Diff 326516.
cryptoad added a comment.

Opt for a version that satisfies `clang-diagnostic-covered-switch-default`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97496

Files:
  clang/lib/Basic/Sanitizers.cpp


Index: clang/lib/Basic/Sanitizers.cpp
===
--- clang/lib/Basic/Sanitizers.cpp
+++ clang/lib/Basic/Sanitizers.cpp
@@ -70,6 +70,7 @@
   case llvm::AsanDtorKind::Invalid:
 return "invalid";
   }
+  return "invalid";
 }
 
 llvm::AsanDtorKind AsanDtorKindFromString(StringRef kindStr) {


Index: clang/lib/Basic/Sanitizers.cpp
===
--- clang/lib/Basic/Sanitizers.cpp
+++ clang/lib/Basic/Sanitizers.cpp
@@ -70,6 +70,7 @@
   case llvm::AsanDtorKind::Invalid:
 return "invalid";
   }
+  return "invalid";
 }
 
 llvm::AsanDtorKind AsanDtorKindFromString(StringRef kindStr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97496: [Clang][ASan] Correct AsanDtorKindToString to return non-void in default case

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
cryptoad added reviewers: delcypher, vitalybuka.
Herald added a subscriber: dexonsmith.
cryptoad requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Post D96572 , a warning started showing up for 
me:
`clang/lib/Basic/Sanitizers.cpp:73:1: warning: control reaches end of non-void 
function [-Wreturn-type]`

So this adds a default to the case to return invalid, which seems appropriate,
and appears to correct the issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97496

Files:
  clang/lib/Basic/Sanitizers.cpp


Index: clang/lib/Basic/Sanitizers.cpp
===
--- clang/lib/Basic/Sanitizers.cpp
+++ clang/lib/Basic/Sanitizers.cpp
@@ -68,6 +68,8 @@
   case llvm::AsanDtorKind::Global:
 return "global";
   case llvm::AsanDtorKind::Invalid:
+LLVM_FALLTHROUGH;
+  default:
 return "invalid";
   }
 }


Index: clang/lib/Basic/Sanitizers.cpp
===
--- clang/lib/Basic/Sanitizers.cpp
+++ clang/lib/Basic/Sanitizers.cpp
@@ -68,6 +68,8 @@
   case llvm::AsanDtorKind::Global:
 return "global";
   case llvm::AsanDtorKind::Invalid:
+LLVM_FALLTHROUGH;
+  default:
 return "invalid";
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92413: Argument dependent lookup with class argument is recursing into base classes that haven't been instantiated. This is generating an assertion in DeclTemplate.h. Fix for Bug25668.

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad abandoned this revision.
cryptoad added a comment.

This is an arc fail on my side


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92413

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

[PATCH] D92413: Argument dependent lookup with class argument is recursing into base classes that haven't been instantiated. This is generating an assertion in DeclTemplate.h. Fix for Bug25668.

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
cryptoad requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92413

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/template-specialization.cpp


Index: clang/test/OpenMP/template-specialization.cpp
===
--- /dev/null
+++ clang/test/OpenMP/template-specialization.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -verify -fopenmp -fsyntax-only %s
+
+// expected-no-diagnostics
+
+template 
+struct z {
+  static void aj() {
+T f;
+#pragma omp target map(f)
+;
+  }
+};
+
+template  class ar {};
+template  struct as {};
+template class z>>;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -17549,6 +17549,7 @@
 auto  = SemaRef.getASTContext().DeclarationNames;
 MapperId.setName(DeclNames.getIdentifier(
 ().Idents.get("default")));
+MapperId.setLoc(StartLoc);
   }
 
   // Iterators to find the current unresolved mapper expression.
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -2576,6 +2576,8 @@
 
 bool addClassTransitive(CXXRecordDecl *RD) {
   Classes.insert(RD);
+  if (InstantiationLoc.isInvalid())
+InstantiationLoc = RD->getLocation();
   return ClassesTransitive.insert(RD);
 }
 


Index: clang/test/OpenMP/template-specialization.cpp
===
--- /dev/null
+++ clang/test/OpenMP/template-specialization.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -verify -fopenmp -fsyntax-only %s
+
+// expected-no-diagnostics
+
+template 
+struct z {
+  static void aj() {
+T f;
+#pragma omp target map(f)
+;
+  }
+};
+
+template  class ar {};
+template  struct as {};
+template class z>>;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -17549,6 +17549,7 @@
 auto  = SemaRef.getASTContext().DeclarationNames;
 MapperId.setName(DeclNames.getIdentifier(
 ().Idents.get("default")));
+MapperId.setLoc(StartLoc);
   }
 
   // Iterators to find the current unresolved mapper expression.
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -2576,6 +2576,8 @@
 
 bool addClassTransitive(CXXRecordDecl *RD) {
   Classes.insert(RD);
+  if (InstantiationLoc.isInvalid())
+InstantiationLoc = RD->getLocation();
   return ClassesTransitive.insert(RD);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

Thank you for all your work!
IIRC everything was working except as you point out some of the very Unixy 
tests.
Is your build creating a `clang_rt.scudo_cxx` library that also needs to be 
link as well? The C++ specific interceptors usually end up in a separate 
library that is not linked into the straight C programs, which is the most 
likely culprit for not having new/delete be intercepted.

In D86694#2360026 , @russell.gallop 
wrote:

> These are all down to new/delete not being replaced by the scudo versions, so 
> scudoAllocate and scudoDeallocate don't get the appropriate size and Type 
> parameters. The cxx library should be linked by SanitizerArgs::addArgs so I'm 
> not sure why this is. @cryptoad, it looks like you did some work in 
> scudo_new_delete.cpp, did you have this working?
>
> Does new/delete need to work for this, or should I mark these tests as 
> unsupported on Windows for the purposes of a step towards getting the 
> standalone scudo working?
>
> Stage 2 builds with 
> -DLLVM_INTEGRATED_CRT_ALLOC=/stage1/lib/clang/12.0.0/lib/windows/clang_rt.scudo-x86_64.lib
>  -DLLVM_USE_CRT_RELEASE=MT.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2287242 , @russell.gallop 
wrote:

> I'm considering a slight change of plan. @cryptoad, in the name of 
> incremental development, would you be happy for me to put the scudo sanitizer 
> version up (with working tests), or is this deprecated now? It may not be the 
> best for performance long term but it's a smaller step than the standalone 
> port and seems to work.

Sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2277371 , @aganea wrote:

> @cryptoad What happens if the primary was much smaller? Or if pages were 
> //reserved// in much smaller ranges?

The Primary can be made smaller, but this works better with the Standalone 
version as some code was added to fallback to larger class sizes if a region is 
full (Android uses 256mb per region).

> @cryptoad Does SCUDO standalone differs in any of these aspects from this 
> version?

So this requires a bit of background.
There are two models for the Thread Specific Data that holds the cached 
pointers: Shared (a pool of N caches is shared between all threads) and 
Exclusive (1 exclusive cache per thread).
For my initial port to Windows, I used the Shared model, with a pool of 32 
caches max (it's a define in the platform header). If there is more than 32 
cores, this can be increased.
I didn't try to make the Exclusive version work, mostly because I was using the 
Windows TLS API and the Shared fit right in with those, but it would get rid of 
a lot of the contention.

Overall with regard to the Standalone, it should be better on all accounts: 
faster (as we got rid of some of the quirks of sanitizer_common), lesser memory 
footprint, better reclaiming, more configurable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2274548 , @russell.gallop 
wrote:

> I guess using scudo as a general purpose allocator that could set a limit on 
> the number of cores that can be used at once as @aganea found. Would there be 
> any problem with making this very small (e.g. a couple of GB)?

You can reduce the size of the Primary for Windows with an additional define in 
the platform file. You probably want to make sure there is at least a few gigs 
per region (eg: the total size could be 256G).

Once again the memory is reserved but not committed, and this is on a 
per-process basis. There shouldn't be more than one Primary, and as such we 
shouldn't run out of VA space. We could possibly run out of memory if we 
allocate past the amount of RAM (+swap), but this is the committed amount.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2273815 , @aganea wrote:

> If 4.4 TB of virtual pages are mapped in each process (this happens on 
> startup), then we quickly exaust the 48-bit (256 TB) addressable space with 
> 72+ programs running (on a 36-core). Any idea where this 4.4 TB mapping comes 
> from?

The size of the Primary is defined in 
https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/scudo/scudo_platform.h#L75
Scudo reserves that size but doesn't commit it, then it incrementally commits 
when memory is needed within the reserved region.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2268182 , @russell.gallop 
wrote:

> 



> Thanks. I'm looking at porting the standalone variant, drawing on D42519 
>  and Windows support from sanitizer_common. 
> Does that sound like a reasonable approach?

Yup, the structure is a tad different, and other platforms (Fuchsia, Linux cpp) 
have some implementation of the platform specific functions.

> Would these be good flags to use:
>
>   set 
> SCUDO_OPTIONS=allocator_release_to_os_interval_ms=-1:QuarantineSizeKb=0:ThreadLocalQuarantineSizeKb=0:DeleteSizeMismatch=0:DeallocationTypeMismatch=0

Yup, no release, no quarantine is a great. Size class map compile time options 
could also make a difference.


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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D86694#2242150 , @russell.gallop 
wrote:

> In D86694#2242140 , @cryptoad wrote:
>
>> That's awesome! Is it meant to eventually be committed or only be used for 
>> comparison purposes?
>
> I'd like it to be committed, but can't claim I know the code from 
> https://reviews.llvm.org/D42519 well enough. The good news is that I can 
> build LLVM on Windows with this. Is there a good sanity check that it is 
> actually using Scudo rather than silently using the standard alloc?

Nothing except the tests. Compiling a sizeable application with Scudo as well.

> You marked D42519  as WIP, can you remember 
> what was still TBD?

Mostly because I didn't have the time and resources to make sure this would 
work over the long term.
I used my home windows box to do this as a proof of concept that it could be 
done.

> Also, it might make sense to separate out the "use" of sanitize=Scudo in 
> LLVM, from providing Windows support. I put them together here for evaluation 
> purposes.

Yeah this makes sense.

The other point that is worth mentioning is that we moved all dev efforts to 
the "standalone" version of Scudo (eg: the one not depending on 
sanitizer_common in the standalone/ subdirectory).
There is enough differences that there could be some significant 
performance/mem footprint changes between the 2.

Also depending on what you want to compare, disabling the Quarantine and other 
optional security features will make things faster and use less memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

That's awesome! Is it meant to eventually be committed or only be used for 
comparison purposes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D62368: Add vendor identity check for Hygon Dhyana processor in Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D62368#2030005 , @craig.topper 
wrote:

> I've deleted the branch in the github web interface. I didn't know I could do 
> that.


Ok thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368



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

[PATCH] D62368: Add vendor identity check for Hygon Dhyana processor in Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D62368#2029752 , @craig.topper 
wrote:

> I noticed when I pulled this morning that this seems to have been pushed as a 
> branch to the repo
>
> From https://github.com/llvm/llvm-project
>
> eb7d32e..63a4fdd  master -> origin/master
>   * [new branch]  arcpatch-D62368 -> origin/arcpatch-D62368


Hey Craig, I gave a try to arc land this morning as opposed to my regular git 
llvm push workflow.
The initial attempt failed at some point so I went back to git llvm push which 
succeeded 
(https://github.com/llvm/llvm-project/commit/9959eb918acfd37ce89d4a7082ac9957d3628f16#diff-483266ff26e7d1b17130f1b371c4e62d).
Is there something I have to clean up somewhere for that other branch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368



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

[PATCH] D62368: Add vendor identity check for Hygon Dhyana processor in Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9959eb918acf: Add vendor identity check for Hygon Dhyana 
processor in Scudo (authored by cryptoad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368

Files:
  compiler-rt/lib/scudo/scudo_utils.cpp
  compiler-rt/lib/scudo/standalone/checksum.cpp


Index: compiler-rt/lib/scudo/standalone/checksum.cpp
===
--- compiler-rt/lib/scudo/standalone/checksum.cpp
+++ compiler-rt/lib/scudo/standalone/checksum.cpp
@@ -31,6 +31,13 @@
 #define bit_SSE4_2 bit_SSE42 // clang and gcc have different defines.
 #endif
 
+#ifndef signature_HYGON_ebx // They are not defined in gcc.
+// HYGON: "HygonGenuine".
+#define signature_HYGON_ebx 0x6f677948
+#define signature_HYGON_edx 0x6e65476e
+#define signature_HYGON_ecx 0x656e6975
+#endif
+
 bool hasHardwareCRC32() {
   u32 Eax, Ebx = 0, Ecx = 0, Edx = 0;
   __get_cpuid(0, , , , );
@@ -39,7 +46,10 @@
(Ecx == signature_INTEL_ecx);
   const bool IsAMD = (Ebx == signature_AMD_ebx) && (Edx == signature_AMD_edx) 
&&
  (Ecx == signature_AMD_ecx);
-  if (!IsIntel && !IsAMD)
+  const bool IsHygon = (Ebx == signature_HYGON_ebx) &&
+   (Edx == signature_HYGON_edx) &&
+   (Ecx == signature_HYGON_ecx);
+  if (!IsIntel && !IsAMD && !IsHygon)
 return false;
   __get_cpuid(1, , , , );
   return !!(Ecx & bit_SSE4_2);
Index: compiler-rt/lib/scudo/scudo_utils.cpp
===
--- compiler-rt/lib/scudo/scudo_utils.cpp
+++ compiler-rt/lib/scudo/scudo_utils.cpp
@@ -62,6 +62,14 @@
 # ifndef bit_SSE4_2
 #  define bit_SSE4_2 bit_SSE42  // clang and gcc have different defines.
 # endif
+
+#ifndef signature_HYGON_ebx // They are not defined in gcc.
+// HYGON: "HygonGenuine".
+#define signature_HYGON_ebx 0x6f677948
+#define signature_HYGON_edx 0x6e65476e
+#define signature_HYGON_ecx 0x656e6975
+#endif
+
 bool hasHardwareCRC32() {
   u32 Eax, Ebx, Ecx, Edx;
   __get_cpuid(0, , , , );
@@ -71,7 +79,10 @@
   const bool IsAMD = (Ebx == signature_AMD_ebx) &&
  (Edx == signature_AMD_edx) &&
  (Ecx == signature_AMD_ecx);
-  if (!IsIntel && !IsAMD)
+  const bool IsHygon = (Ebx == signature_HYGON_ebx) &&
+   (Edx == signature_HYGON_edx) &&
+   (Ecx == signature_HYGON_ecx);
+  if (!IsIntel && !IsAMD && !IsHygon)
 return false;
   __get_cpuid(1, , , , );
   return !!(Ecx & bit_SSE4_2);


Index: compiler-rt/lib/scudo/standalone/checksum.cpp
===
--- compiler-rt/lib/scudo/standalone/checksum.cpp
+++ compiler-rt/lib/scudo/standalone/checksum.cpp
@@ -31,6 +31,13 @@
 #define bit_SSE4_2 bit_SSE42 // clang and gcc have different defines.
 #endif
 
+#ifndef signature_HYGON_ebx // They are not defined in gcc.
+// HYGON: "HygonGenuine".
+#define signature_HYGON_ebx 0x6f677948
+#define signature_HYGON_edx 0x6e65476e
+#define signature_HYGON_ecx 0x656e6975
+#endif
+
 bool hasHardwareCRC32() {
   u32 Eax, Ebx = 0, Ecx = 0, Edx = 0;
   __get_cpuid(0, , , , );
@@ -39,7 +46,10 @@
(Ecx == signature_INTEL_ecx);
   const bool IsAMD = (Ebx == signature_AMD_ebx) && (Edx == signature_AMD_edx) &&
  (Ecx == signature_AMD_ecx);
-  if (!IsIntel && !IsAMD)
+  const bool IsHygon = (Ebx == signature_HYGON_ebx) &&
+   (Edx == signature_HYGON_edx) &&
+   (Ecx == signature_HYGON_ecx);
+  if (!IsIntel && !IsAMD && !IsHygon)
 return false;
   __get_cpuid(1, , , , );
   return !!(Ecx & bit_SSE4_2);
Index: compiler-rt/lib/scudo/scudo_utils.cpp
===
--- compiler-rt/lib/scudo/scudo_utils.cpp
+++ compiler-rt/lib/scudo/scudo_utils.cpp
@@ -62,6 +62,14 @@
 # ifndef bit_SSE4_2
 #  define bit_SSE4_2 bit_SSE42  // clang and gcc have different defines.
 # endif
+
+#ifndef signature_HYGON_ebx // They are not defined in gcc.
+// HYGON: "HygonGenuine".
+#define signature_HYGON_ebx 0x6f677948
+#define signature_HYGON_edx 0x6e65476e
+#define signature_HYGON_ecx 0x656e6975
+#endif
+
 bool hasHardwareCRC32() {
   u32 Eax, Ebx, Ecx, Edx;
   __get_cpuid(0, , , , );
@@ -71,7 +79,10 @@
   const bool IsAMD = (Ebx == signature_AMD_ebx) &&
  (Edx == signature_AMD_edx) &&
  (Ecx == signature_AMD_ecx);
-  if (!IsIntel && !IsAMD)
+  const bool IsHygon = (Ebx == signature_HYGON_ebx) &&
+   (Edx == signature_HYGON_edx) &&
+   (Ecx == signature_HYGON_ecx);
+  if (!IsIntel && !IsAMD && !IsHygon)
 return false;
   __get_cpuid(1, , , , );
   return !!(Ecx & bit_SSE4_2);
___
cfe-commits 

[PATCH] D62368: Add vendor identity check for Hygon Dhyana processor in Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D62368#2027895 , @fanjinke wrote:

> Hi Cryptoad，
>
> Could you help me to commit the patch? Because I don't have access.
>
> Thanks.


Done, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368



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

[PATCH] D62368: Add vendor identity check for Hygon Dhyana processor in Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

A couple of style issues to address.




Comment at: compiler-rt/lib/scudo/scudo_utils.cpp:66
+
+#ifndef signature_HYGON_ebx // They are not defined in the gcc.
+/* HYGON:   "HygonGenuine" */

s/the gcc/gcc/



Comment at: compiler-rt/lib/scudo/scudo_utils.cpp:67
+#ifndef signature_HYGON_ebx // They are not defined in the gcc.
+/* HYGON:   "HygonGenuine" */
+#define signature_HYGON_ebx 0x6f677948

Please use C++-style comments 
(https://llvm.org/docs/CodingStandards.html#comment-formatting)



Comment at: compiler-rt/lib/scudo/standalone/checksum.cpp:34
 
+#ifndef signature_HYGON_ebx // They are not defined in the gcc.
+/* HYGON:   "HygonGenuine" */

s/the gcc/gcc



Comment at: compiler-rt/lib/scudo/standalone/checksum.cpp:35
+#ifndef signature_HYGON_ebx // They are not defined in the gcc.
+/* HYGON:   "HygonGenuine" */
+#define signature_HYGON_ebx 0x6f677948

Please use C++-style comments 
(https://llvm.org/docs/CodingStandards.html#comment-formatting)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368



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

[PATCH] D62368: Add support for Hygon Dhyana processor

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

Hey,

`clang/lib/Headers/cpuid.h` would have to be in its own CL that would have to 
be sent separately from the Scudo one.
It would have to be reviewed by clang people and likely some tests added.

Once this is done and landed, then the Scudo part can happen.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368



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

[PATCH] D62368: Add support for Hygon Dhyana processor

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

Regarding the Scudo side of the patch: the code has to be able to compile with 
gcc as well, and not necessarily the latest version.
This won't compile on systems without a `signature_HYGON_*`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62368



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

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

In D61923#1502337 , @eugenis wrote:

> I think the idea is that implementing our own spinlock is not much harder 
> than having 3 platform-specific implementations (posix, window, fuchsia).
>  Also, scudo_standalone is doing the same (  @cryptoad, why? ).
>
> As Mitch mentioned, we should move the implementation into a common 
> directory. Why not do this now?


Is the question why do Scudo has its own as opposed to rely on platform 
specific ones?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61923



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

[PATCH] D48833: [Driver] Add PPC64 as supported for Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336202: [Driver] Add PPC64 as supported for Scudo (authored 
by cryptoad, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48833?vs=153727=153915#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48833

Files:
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/fsanitize.c


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -670,6 +670,8 @@
 // RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 
| FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1 
| FileCheck %s --check-prefix=CHECK-SCUDO
 // CHECK-SCUDO: "-fsanitize=scudo"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO-PIE
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -931,7 +931,8 @@
 Res |= SanitizerKind::Efficiency;
   if (IsX86 || IsX86_64)
 Res |= SanitizerKind::Function;
-  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+  IsPowerPC64)
 Res |= SanitizerKind::Scudo;
   if (IsX86_64 || IsAArch64) {
 Res |= SanitizerKind::HWAddress;


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -670,6 +670,8 @@
 // RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // CHECK-SCUDO: "-fsanitize=scudo"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-PIE
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -931,7 +931,8 @@
 Res |= SanitizerKind::Efficiency;
   if (IsX86 || IsX86_64)
 Res |= SanitizerKind::Function;
-  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+  IsPowerPC64)
 Res |= SanitizerKind::Scudo;
   if (IsX86_64 || IsAArch64) {
 Res |= SanitizerKind::HWAddress;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48833: [Driver] Add PPC64 as supported for Scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
cryptoad added reviewers: eugenis, alekseyshl.

Scudo works on PPC64 as is, so mark the architecture as supported for it. This
will also require a change to config-ix.cmake on the compiler-rt side.

Update the tests accordingly.


Repository:
  rC Clang

https://reviews.llvm.org/D48833

Files:
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/fsanitize.c


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -670,6 +670,8 @@
 // RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 
| FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1 
| FileCheck %s --check-prefix=CHECK-SCUDO
 // CHECK-SCUDO: "-fsanitize=scudo"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO-PIE
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -931,7 +931,8 @@
 Res |= SanitizerKind::Efficiency;
   if (IsX86 || IsX86_64)
 Res |= SanitizerKind::Function;
-  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+  IsPowerPC64)
 Res |= SanitizerKind::Scudo;
   if (IsX86_64 || IsAArch64) {
 Res |= SanitizerKind::HWAddress;


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -670,6 +670,8 @@
 // RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // CHECK-SCUDO: "-fsanitize=scudo"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-PIE
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -931,7 +931,8 @@
 Res |= SanitizerKind::Efficiency;
   if (IsX86 || IsX86_64)
 Res |= SanitizerKind::Function;
-  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+  IsPowerPC64)
 Res |= SanitizerKind::Scudo;
   if (IsX86_64 || IsAArch64) {
 Res |= SanitizerKind::HWAddress;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48570: [Driver] Do not add -lpthread & -lrt with -static-libsan on Android

[Kostya Kortchinsky via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC335620: [Driver] Do not add -lpthread  -lrt with 
-static-libsan on Android (authored by cryptoad, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48570?vs=152900=152901#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48570

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -153,7 +153,8 @@
 //
 // CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
-// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
@@ -175,7 +176,8 @@
 //
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: 
libclang_rt.ubsan_standalone-arm-android.a"
-// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -735,7 +737,8 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
 // CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" 
"{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
-// CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -564,8 +564,9 @@
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
-  // There's no libpthread or librt on RTEMS.
-  if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
+  // There's no libpthread or librt on RTEMS & Android.
+  if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
+  !TC.getTriple().isAndroid()) {
 CmdArgs.push_back("-lpthread");
 if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
   CmdArgs.push_back("-lrt");


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -153,7 +153,8 @@
 //
 // CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
-// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
@@ -175,7 +176,8 @@
 //
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: libclang_rt.ubsan_standalone-arm-android.a"
-// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -735,7 +737,8 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
 // CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
-// CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -564,8 +564,9 @@
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
-  // There's no libpthread or librt on RTEMS.
-  if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
+  // There's no libpthread or librt on RTEMS & Android.
+  if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
+  !TC.getTriple().isAndroid()) {
 CmdArgs.push_back("-lpthread");
 if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
   CmdArgs.push_back("-lrt");

[PATCH] D48570: [Driver] Do not add -lpthread & -lrt with -static-libsan on Android

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad updated this revision to Diff 152900.
cryptoad added a comment.

Rebasing.


Repository:
  rC Clang

https://reviews.llvm.org/D48570

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -153,7 +153,8 @@
 //
 // CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
-// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
@@ -175,7 +176,8 @@
 //
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: 
libclang_rt.ubsan_standalone-arm-android.a"
-// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -735,7 +737,8 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
 // CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" 
"{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
-// CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -564,8 +564,9 @@
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
-  // There's no libpthread or librt on RTEMS.
-  if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
+  // There's no libpthread or librt on RTEMS & Android.
+  if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
+  !TC.getTriple().isAndroid()) {
 CmdArgs.push_back("-lpthread");
 if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
   CmdArgs.push_back("-lrt");


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -153,7 +153,8 @@
 //
 // CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
-// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
@@ -175,7 +176,8 @@
 //
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: libclang_rt.ubsan_standalone-arm-android.a"
-// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -735,7 +737,8 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
 // CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
-// CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -564,8 +564,9 @@
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
-  // There's no libpthread or librt on RTEMS.
-  if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
+  // There's no libpthread or librt on RTEMS & Android.
+  if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
+  !TC.getTriple().isAndroid()) {
 CmdArgs.push_back("-lpthread");
 if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
   CmdArgs.push_back("-lrt");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48570: [Driver] Do not add -lpthread & -lrt with -static-libsan on Android

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
cryptoad added reviewers: eugenis, pirama, srhines.

I am not sure anyone has tried to compile an application with sanitizers on
Android with `-static-libsan`, and a recent NDK, but it fails with:

  .../i686-linux-android/bin/ld.gold: error: cannot find -lpthread
  .../i686-linux-android/bin/ld.gold: error: cannot find -lrt

My understanding is that both are included in Bionic and as such are not needed,
and actually error out.

So remove the addition of those two in `linkSanitizerRuntimeDeps` when dealing
with Android, and update the tests.

I am unfamiliar with the evolution of the NDK and I am not sure if this has
always been the case or if this is somewhat of a recent evolution. I'll let
Android people chime in.


Repository:
  rC Clang

https://reviews.llvm.org/D48570

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -153,7 +153,8 @@
 //
 // CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
-// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
@@ -175,7 +176,8 @@
 //
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: 
libclang_rt.ubsan_standalone-arm-android.a"
-// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -735,7 +737,8 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
 // CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" 
"{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
-// CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -558,8 +558,9 @@
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
-  // There's no libpthread or librt on RTEMS.
-  if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
+  // There's no libpthread or librt on RTEMS & Android.
+  if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
+  !TC.getTriple().isAndroid()) {
 CmdArgs.push_back("-lpthread");
 if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
   CmdArgs.push_back("-lrt");


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -153,7 +153,8 @@
 //
 // CHECK-ASAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-ANDROID-STATICLIBASAN: libclang_rt.asan-arm-android.a"
-// CHECK-ASAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-ASAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=undefined \
@@ -175,7 +176,8 @@
 //
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-UBSAN-ANDROID-STATICLIBASAN: libclang_rt.ubsan_standalone-arm-android.a"
-// CHECK-UBSAN-ANDROID-STATICLIBASAN: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lpthread"
+// CHECK-UBSAN-ANDROID-STATICLIBASAN-NOT: "-lrt"
 
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -735,7 +737,8 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
 // CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
-// CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
+// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -558,8 +558,9 @@
   // Force linking against the system libraries sanitizers depends on

[PATCH] D48373: [Driver] Make scudo compatible with -fsanitize-minimal-runtime

[Kostya Kortchinsky via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC335352: [Driver] Make scudo compatible with 
-fsanitize-minimal-runtime (authored by cryptoad, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48373?vs=152171=152476#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48373

Files:
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/fsanitize.c
  test/Driver/sanitizer-ld.c

Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -689,6 +689,15 @@
 // CHECK-SCUDO-LINUX: "-lpthread"
 // CHECK-SCUDO-LINUX: "-ldl"
 
+// RUN: %clang -fsanitize=scudo -fsanitize-minimal-runtime %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux -fuse-ld=ld \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SCUDO-MINIMAL-LINUX %s
+// CHECK-SCUDO-MINIMAL-LINUX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-SCUDO-MINIMAL-LINUX: "-pie"
+// CHECK-SCUDO-MINIMAL-LINUX: "--whole-archive" "{{.*}}libclang_rt.scudo_minimal-i386.a" "--no-whole-archive"
+// CHECK-SCUDO-MINIMAL-LINUX: "-lpthread"
+
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.so -shared 2>&1 \
 // RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=scudo -shared-libsan \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -674,6 +674,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo,undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-UBSAN
 // CHECK-SCUDO-UBSAN: "-fsanitize={{.*}}scudo"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-MINIMAL
+// CHECK-SCUDO-MINIMAL: "-fsanitize=scudo"
+// CHECK-SCUDO-MINIMAL: "-fsanitize-minimal-runtime"
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined,scudo -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-UBSAN-MINIMAL
+// CHECK-SCUDO-UBSAN-MINIMAL: "-fsanitize={{.*}}scudo"
+// CHECK-SCUDO-UBSAN-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target powerpc-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SCUDO
 // CHECK-NO-SCUDO: unsupported option
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -45,7 +45,7 @@
   Nullability | LocalBounds | CFI,
   TrappingDefault = CFI,
   CFIClasses = CFIVCall | CFINVCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
 };
 
 enum CoverageFeature {
@@ -179,7 +179,8 @@
 bool SanitizerArgs::needsUbsanRt() const {
   // All of these include ubsan.
   if (needsAsanRt() || needsMsanRt() || needsHwasanRt() || needsTsanRt() ||
-  needsDfsanRt() || needsLsanRt() || needsCfiDiagRt() || needsScudoRt())
+  needsDfsanRt() || needsLsanRt() || needsCfiDiagRt() ||
+  (needsScudoRt() && !requiresMinimalRuntime()))
 return false;
 
   return (Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -593,14 +593,17 @@
 HelperStaticRuntimes.push_back("asan-preinit");
 }
 if (SanArgs.needsUbsanRt()) {
-  if (SanArgs.requiresMinimalRuntime()) {
+  if (SanArgs.requiresMinimalRuntime())
 SharedRuntimes.push_back("ubsan_minimal");
-  } else {
+  else
 SharedRuntimes.push_back("ubsan_standalone");
-  }
 }
-if (SanArgs.needsScudoRt())
-  SharedRuntimes.push_back("scudo");
+if (SanArgs.needsScudoRt()) {
+  if (SanArgs.requiresMinimalRuntime())
+SharedRuntimes.push_back("scudo_minimal");
+  else
+SharedRuntimes.push_back("scudo");
+}
 if (SanArgs.needsHwasanRt())
   SharedRuntimes.push_back("hwasan");
   }
@@ -666,9 +669,15 @@
   if (SanArgs.needsEsanRt())
 StaticRuntimes.push_back("esan");
   if (SanArgs.needsScudoRt()) {
-StaticRuntimes.push_back("scudo");
-if (SanArgs.linkCXXRuntimes())
-  StaticRuntimes.push_back("scudo_cxx");
+if (SanArgs.requiresMinimalRuntime()) {
+  StaticRuntimes.push_back("scudo_minimal");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx_minimal");
+} else {
+  StaticRuntimes.push_back("scudo");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx");
+}
   }
 }
 
___
cfe-commits 

[PATCH] D48373: [Driver] Make scudo compatible with -fsanitize-minimal-runtime

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad updated this revision to Diff 152171.
cryptoad added a comment.

Adding tests.


Repository:
  rC Clang

https://reviews.llvm.org/D48373

Files:
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/fsanitize.c
  test/Driver/sanitizer-ld.c

Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -689,6 +689,15 @@
 // CHECK-SCUDO-LINUX: "-lpthread"
 // CHECK-SCUDO-LINUX: "-ldl"
 
+// RUN: %clang -fsanitize=scudo -fsanitize-minimal-runtime %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux -fuse-ld=ld \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SCUDO-MINIMAL-LINUX %s
+// CHECK-SCUDO-MINIMAL-LINUX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-SCUDO-MINIMAL-LINUX: "-pie"
+// CHECK-SCUDO-MINIMAL-LINUX: "--whole-archive" "{{.*}}libclang_rt.scudo_minimal-i386.a" "--no-whole-archive"
+// CHECK-SCUDO-MINIMAL-LINUX: "-lpthread"
+
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.so -shared 2>&1 \
 // RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=scudo -shared-libsan \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -674,6 +674,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo,undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-UBSAN
 // CHECK-SCUDO-UBSAN: "-fsanitize={{.*}}scudo"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-MINIMAL
+// CHECK-SCUDO-MINIMAL: "-fsanitize=scudo"
+// CHECK-SCUDO-MINIMAL: "-fsanitize-minimal-runtime"
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined,scudo -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-UBSAN-MINIMAL
+// CHECK-SCUDO-UBSAN-MINIMAL: "-fsanitize={{.*}}scudo"
+// CHECK-SCUDO-UBSAN-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target powerpc-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SCUDO
 // CHECK-NO-SCUDO: unsupported option
 
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -593,14 +593,17 @@
 HelperStaticRuntimes.push_back("asan-preinit");
 }
 if (SanArgs.needsUbsanRt()) {
-  if (SanArgs.requiresMinimalRuntime()) {
+  if (SanArgs.requiresMinimalRuntime())
 SharedRuntimes.push_back("ubsan_minimal");
-  } else {
+  else
 SharedRuntimes.push_back("ubsan_standalone");
-  }
 }
-if (SanArgs.needsScudoRt())
-  SharedRuntimes.push_back("scudo");
+if (SanArgs.needsScudoRt()) {
+  if (SanArgs.requiresMinimalRuntime())
+SharedRuntimes.push_back("scudo_minimal");
+  else
+SharedRuntimes.push_back("scudo");
+}
 if (SanArgs.needsHwasanRt())
   SharedRuntimes.push_back("hwasan");
   }
@@ -666,9 +669,15 @@
   if (SanArgs.needsEsanRt())
 StaticRuntimes.push_back("esan");
   if (SanArgs.needsScudoRt()) {
-StaticRuntimes.push_back("scudo");
-if (SanArgs.linkCXXRuntimes())
-  StaticRuntimes.push_back("scudo_cxx");
+if (SanArgs.requiresMinimalRuntime()) {
+  StaticRuntimes.push_back("scudo_minimal");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx_minimal");
+} else {
+  StaticRuntimes.push_back("scudo");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx");
+}
   }
 }
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -45,7 +45,7 @@
   Nullability | LocalBounds | CFI,
   TrappingDefault = CFI,
   CFIClasses = CFIVCall | CFINVCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
 };
 
 enum CoverageFeature {
@@ -179,7 +179,8 @@
 bool SanitizerArgs::needsUbsanRt() const {
   // All of these include ubsan.
   if (needsAsanRt() || needsMsanRt() || needsHwasanRt() || needsTsanRt() ||
-  needsDfsanRt() || needsLsanRt() || needsCfiDiagRt() || needsScudoRt())
+  needsDfsanRt() || needsLsanRt() || needsCfiDiagRt() ||
+  (needsScudoRt() && !requiresMinimalRuntime()))
 return false;
 
   return (Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48373: [Driver] Make scudo compatible with -fsanitize-minimal-runtime

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
cryptoad added a reviewer: eugenis.
Herald added a subscriber: cfe-commits.

This is the clang side of the change, there is a compiler-rt counterpart.

Scudo works with UBSan using `-fsanitize=scudo,integer` for example, and to do
so it embeds UBSan runtime. This makes it not compatible with the UBSan minimal
runtime, but this is something we want for production purposes.

The idea is to have a Scudo minimal runtime on the compiler-rt side that will
not embed UBSan. This is basically the runtime that is currently in use for
Fuchsia, without coverage, stacktraces or symbolization. With this, Scudo
becomes compatible with `-fsanitize-minimal-runtime`.

If this approach is suitable, I'll add the tests as well, otherwise I am open
to other options.


Repository:
  rC Clang

https://reviews.llvm.org/D48373

Files:
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CommonArgs.cpp


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -593,14 +593,17 @@
 HelperStaticRuntimes.push_back("asan-preinit");
 }
 if (SanArgs.needsUbsanRt()) {
-  if (SanArgs.requiresMinimalRuntime()) {
+  if (SanArgs.requiresMinimalRuntime())
 SharedRuntimes.push_back("ubsan_minimal");
-  } else {
+  else
 SharedRuntimes.push_back("ubsan_standalone");
-  }
 }
-if (SanArgs.needsScudoRt())
-  SharedRuntimes.push_back("scudo");
+if (SanArgs.needsScudoRt()) {
+  if (SanArgs.requiresMinimalRuntime())
+SharedRuntimes.push_back("scudo_minimal");
+  else
+SharedRuntimes.push_back("scudo");
+}
 if (SanArgs.needsHwasanRt())
   SharedRuntimes.push_back("hwasan");
   }
@@ -666,9 +669,15 @@
   if (SanArgs.needsEsanRt())
 StaticRuntimes.push_back("esan");
   if (SanArgs.needsScudoRt()) {
-StaticRuntimes.push_back("scudo");
-if (SanArgs.linkCXXRuntimes())
-  StaticRuntimes.push_back("scudo_cxx");
+if (SanArgs.requiresMinimalRuntime()) {
+  StaticRuntimes.push_back("scudo_minimal");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx_minimal");
+} else {
+  StaticRuntimes.push_back("scudo");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx");
+}
   }
 }
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -45,7 +45,7 @@
   Nullability | LocalBounds | CFI,
   TrappingDefault = CFI,
   CFIClasses = CFIVCall | CFINVCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
 };
 
 enum CoverageFeature {
@@ -179,7 +179,8 @@
 bool SanitizerArgs::needsUbsanRt() const {
   // All of these include ubsan.
   if (needsAsanRt() || needsMsanRt() || needsHwasanRt() || needsTsanRt() ||
-  needsDfsanRt() || needsLsanRt() || needsCfiDiagRt() || needsScudoRt())
+  needsDfsanRt() || needsLsanRt() || needsCfiDiagRt() ||
+  (needsScudoRt() && !requiresMinimalRuntime()))
 return false;
 
   return (Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -593,14 +593,17 @@
 HelperStaticRuntimes.push_back("asan-preinit");
 }
 if (SanArgs.needsUbsanRt()) {
-  if (SanArgs.requiresMinimalRuntime()) {
+  if (SanArgs.requiresMinimalRuntime())
 SharedRuntimes.push_back("ubsan_minimal");
-  } else {
+  else
 SharedRuntimes.push_back("ubsan_standalone");
-  }
 }
-if (SanArgs.needsScudoRt())
-  SharedRuntimes.push_back("scudo");
+if (SanArgs.needsScudoRt()) {
+  if (SanArgs.requiresMinimalRuntime())
+SharedRuntimes.push_back("scudo_minimal");
+  else
+SharedRuntimes.push_back("scudo");
+}
 if (SanArgs.needsHwasanRt())
   SharedRuntimes.push_back("hwasan");
   }
@@ -666,9 +669,15 @@
   if (SanArgs.needsEsanRt())
 StaticRuntimes.push_back("esan");
   if (SanArgs.needsScudoRt()) {
-StaticRuntimes.push_back("scudo");
-if (SanArgs.linkCXXRuntimes())
-  StaticRuntimes.push_back("scudo_cxx");
+if (SanArgs.requiresMinimalRuntime()) {
+  StaticRuntimes.push_back("scudo_minimal");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx_minimal");
+} else {
+  StaticRuntimes.push_back("scudo");
+  if (SanArgs.linkCXXRuntimes())
+StaticRuntimes.push_back("scudo_cxx");
+}
   }
 }
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- 

[PATCH] D45997: [CMake] Pass additional CMake flags in Fuchsia cache files

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:89
   set(RUNTIMES_${target}-fuchsia_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} 
CACHE PATH "")
-  set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
-  set(RUNTIMES_${target}-fuchsia_UNIX 1 CACHE BOOL "")
+  set(RUNTIMES_${target}-fuchsia_COMPILER_RT_ENABLE_WERROR ON CACHE BOOL "")
+  set(RUNTIMES_${target}-fuchsia_COMPILER_RT_TEST_COMPILER_CFLAGS 
${FUCHSIA_${target}_C_FLAGS} CACHE PATH "")

This should probably go away for now if we want to have D46079.
Then I can go through the remaining warnings if that's important to have it and 
fix them.


Repository:
  rC Clang

https://reviews.llvm.org/D45997



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

[PATCH] D42416: [Driver] Add support for mips32 and scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added inline comments.



Comment at: lib/Driver/ToolChains/Linux.cpp:850
+  const bool IsMIPS = getTriple().getArch() == llvm::Triple::mips ||
+getTriple().getArch() == llvm::Triple::mipsel;
   const bool IsMIPS64 = getTriple().getArch() == llvm::Triple::mips64 ||

nit: get rid of 2 extra spaces here.


Repository:
  rC Clang

https://reviews.llvm.org/D42416



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

[PATCH] D42416: [Driver] Add support for mips32 and scudo

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad accepted this revision.
cryptoad added a comment.

Cool thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D42416



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

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

So if I understand correctly, the consensus is to abandon this and rewrite it 
to be part of the clang static analyzer?


https://reviews.llvm.org/D29839



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

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad updated this revision to Diff 88048.
cryptoad added a comment.

Missing line separators.


https://reviews.llvm.org/D29839

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/IstreamOverflowCheck.cpp
  clang-tidy/misc/IstreamOverflowCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-istream-overflow.rst
  test/clang-tidy/misc-istream-overflow.cpp

Index: test/clang-tidy/misc-istream-overflow.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-istream-overflow.cpp
@@ -0,0 +1,90 @@
+// RUN: %check_clang_tidy %s misc-istream-overflow %t
+
+// Quick mock of std::istream and what we need to test our plug-in.
+namespace std {
+struct _Setw { int _M_n; };
+inline _Setw setw(int __n) { return { __n }; }
+typedef long int streamsize;
+template
+class basic_istream {
+ public:
+  typedef basic_istream<_CharT, _Traits> __istream_type;
+  basic_istream();
+  ~basic_istream();
+  bool eof() const { return false; }
+  streamsize width() const { return _M_width; }
+  streamsize width(streamsize __wide) {
+streamsize __old = _M_width;
+_M_width = __wide;
+return __old;
+  }
+ protected:
+  streamsize _M_width;
+};
+template struct char_traits {};
+template >
+  class basic_istream;
+typedef basic_istream istream;
+  template
+basic_istream<_CharT, _Traits>&
+operator>>(basic_istream<_CharT, _Traits>& __in, _CharT* __s)
+{ return __in; }
+template
+  inline basic_istream<_CharT, _Traits>&
+  operator>>(basic_istream<_CharT, _Traits>& __is, _Setw __f)
+  {
+__is.width(__f._M_n);
+return __is;
+  }
+}
+
+void bad_1(std::istream ) {
+  char s[8];
+  is >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: istream::operator>> used without setting width
+}
+
+void bad_2(std::istream ) {
+  char s[8];
+  is.width(16);
+  is >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: width set for istream::operator>> (16) is greater than the destination buffer capacity (8)
+}
+
+void bad_3(std::istream ) {
+  char s[8];
+  is >> std::setw(16) >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: width set for istream::operator>> (16) is greater than the destination buffer capacity (8)
+}
+
+// The stream's width is reset to 0 at the end of operator>>, it is typically
+// not sufficient to set it prior to a loop, but it should be set within.
+void bad_3() {
+  std::istream is;
+  char s[8];
+  is.width(8);
+  while (!is.eof()) {
+is >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: istream::operator>> used without setting width
+  }
+}
+
+void good_1(std::istream ) {
+  char s[8];
+  is.width(8);
+  is >> s;
+}
+
+void good_2(std::istream ) {
+  char s[8];
+  is >> std::setw(8) >> s;
+}
+
+void good_3() {
+  std::istream is;
+  char s[8];
+  while (!is.eof()) {
+is.width(8);
+is >> s;
+  }
+}
Index: docs/clang-tidy/checks/misc-istream-overflow.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-istream-overflow.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - misc-istream-overflow
+
+misc-istream-overflow
+=
+
+This check finds calls to ``istream::operator>>`` (and derived classes) into a
+character buffer, that haven't set previously a width. This could result in a
+buffer overflow as the function will keep on reading until it reaches a space
+or EOF. If it finds an operation setting the width of the stream, the check
+will attempt to verify that the size fits the destination buffer.
+
+There are several ways to not have this problem surface:
+
+- call the ``width`` member function prior to using ``operator>>``, this will
+  ensure that only the given number of characters will be stored in the
+  destination buffer. Note that the width of the stream is reset to 0 after
+  each call to ``operator>>``, hence it must be set repeatedly if in a loop for
+  example.
+
+- use ``std::setw``, which in turns will set the width of the stream.
+
+- use an ``std::string`` as a destination instead of a character array, as this
+  will prevent any possibility of overflow.
+
+Given:
+
+.. code-block:: c++
+
+  std::istream is;
+  char s[8];
+
+It will trigger on:
+
+.. code-block:: c++
+
+  // The stream's width hasn't been set
+  is >> s;
+
+but not on:
+
+.. code-block:: c++
+
+  // The stream's width has been set through width()
+  is.width(8);
+  is >> s;
+
+  // The stream's width has been set through std::setw()
+  is >> std::setw(8) >> s;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -66,6 +66,7 @@
misc-inaccurate-erase
misc-incorrect-roundings
misc-inefficient-algorithm
+   misc-istream-overflow
misc-macro-parentheses
misc-macro-repeated-side-effects
misc-misplaced-const
Index: clang-tidy/misc/MiscTidyModule.cpp

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad updated this revision to Diff 88047.
cryptoad added a comment.

Addressing first batch of comments.


https://reviews.llvm.org/D29839

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/IstreamOverflowCheck.cpp
  clang-tidy/misc/IstreamOverflowCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-istream-overflow.rst
  test/clang-tidy/misc-istream-overflow.cpp

Index: test/clang-tidy/misc-istream-overflow.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-istream-overflow.cpp
@@ -0,0 +1,90 @@
+// RUN: %check_clang_tidy %s misc-istream-overflow %t
+
+// Quick mock of std::istream and what we need to test our plug-in.
+namespace std {
+struct _Setw { int _M_n; };
+inline _Setw setw(int __n) { return { __n }; }
+typedef long int streamsize;
+template
+class basic_istream {
+ public:
+  typedef basic_istream<_CharT, _Traits> __istream_type;
+  basic_istream();
+  ~basic_istream();
+  bool eof() const { return false; }
+  streamsize width() const { return _M_width; }
+  streamsize width(streamsize __wide) {
+streamsize __old = _M_width;
+_M_width = __wide;
+return __old;
+  }
+ protected:
+  streamsize _M_width;
+};
+template struct char_traits {};
+template >
+  class basic_istream;
+typedef basic_istream istream;
+  template
+basic_istream<_CharT, _Traits>&
+operator>>(basic_istream<_CharT, _Traits>& __in, _CharT* __s)
+{ return __in; }
+template
+  inline basic_istream<_CharT, _Traits>&
+  operator>>(basic_istream<_CharT, _Traits>& __is, _Setw __f)
+  {
+__is.width(__f._M_n);
+return __is;
+  }
+}
+
+void bad_1(std::istream ) {
+  char s[8];
+  is >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: istream::operator>> used without setting width
+}
+
+void bad_2(std::istream ) {
+  char s[8];
+  is.width(16);
+  is >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: width set for istream::operator>> (16) is greater than the destination buffer capacity (8)
+}
+
+void bad_3(std::istream ) {
+  char s[8];
+  is >> std::setw(16) >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: width set for istream::operator>> (16) is greater than the destination buffer capacity (8)
+}
+
+// The stream's width is reset to 0 at the end of operator>>, it is typically
+// not sufficient to set it prior to a loop, but it should be set within.
+void bad_3() {
+  std::istream is;
+  char s[8];
+  is.width(8);
+  while (!is.eof()) {
+is >> s;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: istream::operator>> used without setting width
+  }
+}
+
+void good_1(std::istream ) {
+  char s[8];
+  is.width(8);
+  is >> s;
+}
+
+void good_2(std::istream ) {
+  char s[8];
+  is >> std::setw(8) >> s;
+}
+
+void good_3() {
+  std::istream is;
+  char s[8];
+  while (!is.eof()) {
+is.width(8);
+is >> s;
+  }
+}
Index: docs/clang-tidy/checks/misc-istream-overflow.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-istream-overflow.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - misc-istream-overflow
+
+misc-istream-overflow
+=
+
+This check finds calls to ``istream::operator>>`` (and derived classes) into a
+character buffer, that haven't set previously a width. This could result in a
+buffer overflow as the function will keep on reading until it reaches a space
+or EOF. If it finds an operation setting the width of the stream, the check
+will attempt to verify that the size fits the destination buffer.
+
+There are several ways to not have this problem surface:
+
+- call the ``width`` member function prior to using ``operator>>``, this will
+  ensure that only the given number of characters will be stored in the
+  destination buffer. Note that the width of the stream is reset to 0 after
+  each call to ``operator>>``, hence it must be set repeatedly if in a loop for
+  example.
+
+- use ``std::setw``, which in turns will set the width of the stream.
+
+- use an ``std::string`` as a destination instead of a character array, as this
+  will prevent any possibility of overflow.
+
+Given:
+
+.. code-block:: c++
+
+  std::istream is;
+  char s[8];
+
+It will trigger on:
+
+.. code-block:: c++
+  // The stream's width hasn't been set
+  is >> s;
+
+but not on:
+
+.. code-block:: c++
+  // The stream's width has been set through width()
+  is.width(8);
+  is >> s;
+
+  // The stream's width has been set through std::setw()
+  is >> std::setw(8) >> s;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -66,6 +66,7 @@
misc-inaccurate-erase
misc-incorrect-roundings
misc-inefficient-algorithm
+   misc-istream-overflow
misc-macro-parentheses
misc-macro-repeated-side-effects
misc-misplaced-const
Index: clang-tidy/misc/MiscTidyModule.cpp

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad marked 5 inline comments as done.
cryptoad added inline comments.



Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:78-80
+if (!Arg->isIntegerConstantExpr(WidthSize, Context)) {
+  llvm::errs() << "Couldn't get width() size.\n";
+}

Prazek wrote:
> debug?
Oops!
I'd like to somehow keep that in there (for debug purposes). Any preferred way> 
(ifdef DEBUG or other).


Repository:
  rL LLVM

https://reviews.llvm.org/D29839



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

[PATCH] D28530: [Driver] Correct empty files in D28238

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad abandoned this revision.
cryptoad added a comment.

Nevermind, it seems to be working with empty files!


https://reviews.llvm.org/D28530



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

[PATCH] D28530: [Driver] Correct empty files in D28238

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
cryptoad added a reviewer: rengolin.
cryptoad added a subscriber: cfe-commits.

The binary files added as part of https://reviews.llvm.org/rL291598 seem to be
empty. Those are the actual files that should have been uploaded by arc.


https://reviews.llvm.org/D28530

Files:
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/crt1.o
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/crti.o
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/crtn.o
  
test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/aarch64-suse-linux/4.8/crtbegin.o
  
test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/aarch64-suse-linux/4.8/crtend.o




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

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad marked 4 inline comments as done.
cryptoad added a comment.

Thank you for the review Renato!


https://reviews.llvm.org/D28238



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

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added inline comments.



Comment at: test/Driver/linux-ld.c:623
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=arm64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \

rengolin wrote:
> cryptoad wrote:
> > rengolin wrote:
> > > shouldn't you have used your triple?
> > The similar Fedora and Ubuntu tests appear to be using an unknown target as 
> > well, I based myself on that.
> This is odd... Just be sure the test doesn't pass without your patch.
Confirming that without the patch the test fails, with the following (slightly 
sanitized) output:
```
Command Output (stderr):
--
[...]/llvm/tools/clang/test/Driver/linux-ld.c:633:33: error: expected string 
not found in input
// CHECK-OPENSUSE-42-2-AARCH64: 
"{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64{{/|}}crt1.o"
^
:6:127: note: scanning from here
 "/usr/bin/ld" 
"--sysroot=[...]/llvm/tools/clang/test/Driver/Inputs/opensuse_42.2_aarch64_tree"
 "
-z" "relro" "--hash-style=gnu" "--eh-frame-hdr" "-m" "aarch64linux" 
"-dynamic-linker" "/lib/ld-linux-aarch64.so.1" "-o" 
"[...]/llvm-build/clang/tools/clang/test/Driver/Output/linux-ld.c.tmp.o" 
"crt1.o" "crti.o" "crtbegin.o" "/tmp
/lit_tmp_V096EA/linux-ld-7a7333.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no
-as-needed" "crtend.o" "crtn.o"
```
Tests pass with the patch.



https://reviews.llvm.org/D28238



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

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added inline comments.



Comment at: test/Driver/linux-ld.c:623
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=arm64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \

rengolin wrote:
> shouldn't you have used your triple?
The similar Fedora and Ubuntu tests appear to be using an unknown target as 
well, I based myself on that.


https://reviews.llvm.org/D28238



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

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad updated this revision to Diff 83376.
cryptoad added a comment.

Adding tests to test/Driver.


https://reviews.llvm.org/D28238

Files:
  lib/Driver/ToolChains.cpp
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/crt1.o
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/crti.o
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/crtn.o
  test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/
  
test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/aarch64-suse-linux/
  
test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/aarch64-suse-linux/4.8/
  
test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/aarch64-suse-linux/4.8/crtbegin.o
  
test/Driver/Inputs/opensuse_42.2_aarch64_tree/usr/lib64/gcc/aarch64-suse-linux/4.8/crtend.o
  test/Driver/linux-ld.c


Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -618,6 +618,26 @@
 // CHECK-SUSE-10-3-PPC64: "-L[[SYSROOT]]/lib/../lib64"
 // CHECK-SUSE-10-3-PPC64: "-L[[SYSROOT]]/usr/lib/../lib64"
 //
+// Check openSuse Leap 42.2 on AArch64
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=arm64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/opensuse_42.2_aarch64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OPENSUSE-42-2-AARCH64 %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/opensuse_42.2_aarch64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OPENSUSE-42-2-AARCH64 %s
+// CHECK-OPENSUSE-42-2-AARCH64: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64{{/|}}crt1.o"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64{{/|}}crti.o"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8{{/|}}crtbegin.o"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"-L[[SYSROOT]]/usr/lib64/gcc/aarch64-suse-linux/4.8"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"-L[[SYSROOT]]/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8{{/|}}crtend.o"
+// CHECK-OPENSUSE-42-2-AARCH64: 
"{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64{{/|}}crtn.o"
+//
 // Check dynamic-linker for different archs
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-gnueabi \
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1531,7 +1531,7 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-linux-android",
-  "aarch64-redhat-linux"};
+  "aarch64-redhat-linux", "aarch64-suse-linux"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};


Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -618,6 +618,26 @@
 // CHECK-SUSE-10-3-PPC64: "-L[[SYSROOT]]/lib/../lib64"
 // CHECK-SUSE-10-3-PPC64: "-L[[SYSROOT]]/usr/lib/../lib64"
 //
+// Check openSuse Leap 42.2 on AArch64
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=arm64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/opensuse_42.2_aarch64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OPENSUSE-42-2-AARCH64 %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/opensuse_42.2_aarch64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OPENSUSE-42-2-AARCH64 %s
+// CHECK-OPENSUSE-42-2-AARCH64: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-OPENSUSE-42-2-AARCH64: "{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64{{/|}}crt1.o"
+// CHECK-OPENSUSE-42-2-AARCH64: "{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64{{/|}}crti.o"
+// CHECK-OPENSUSE-42-2-AARCH64: "{{.*}}/usr/lib64/gcc/aarch64-suse-linux/4.8{{/|}}crtbegin.o"
+// CHECK-OPENSUSE-42-2-AARCH64: "-L[[SYSROOT]]/usr/lib64/gcc/aarch64-suse-linux/4.8"
+// CHECK-OPENSUSE-42-2-AARCH64: "-L[[SYSROOT]]/usr/lib64/gcc/aarch64-suse-linux/4.8/../../../../lib64"
+// 

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

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad added a comment.

Hey Michal,
If I understand correctly, my next move is to move the CRC32 code into it's own 
file an only enable SSE 4.2 for this file?
Shouldn't COMPILER_RT_HAS_MSSE4_2_FLAG be kept for that purpose or is there an 
alternative way to do it?
Thanks


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] D28238: [Driver] Add openSuse AArch64 Triple

[Kostya Kortchinsky via Phabricator via cfe-commits]

cryptoad created this revision.
cryptoad added reviewers: chandlerc, bruno, bkramer.
cryptoad added a subscriber: cfe-commits.
Herald added subscribers: rengolin, aemerson.

openSuse has AArch64 support, with images running on the Raspberry Pi 3.
The libraries and headers live under the aarch64-suse-linux subdirectory,
which is currently not in the AArch64 triples list. Address this by adding
the corresponding string to AArch64Triples.


https://reviews.llvm.org/D28238

Files:
  lib/Driver/ToolChains.cpp


Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1531,7 +1531,7 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-linux-android",
-  "aarch64-redhat-linux"};
+  "aarch64-redhat-linux", "aarch64-suse-linux"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};


Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1531,7 +1531,7 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-linux-android",
-  "aarch64-redhat-linux"};
+  "aarch64-redhat-linux", "aarch64-suse-linux"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits