<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 5, 2017 at 10:28 AM Maxim Ostapenko <<a href="mailto:chefmax7@gmail.com">chefmax7@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 05/06/17 20:16, David Blaikie wrote:<br>
> This change seemed to be buggy (& I assume missing test coverage to<br>
> demonstrate that). Galina fixed it in<br>
> <a href="http://llvm.org/viewvc/llvm-project?rev=304663&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=304663&view=rev</a> based on a<br>
> compiler warning.<br>
<br>
Oh, right, I've missed that. Sorry!<br>
<br>
><br>
> Can you add test coverage to this code to exercise these untested<br>
> codepaths?<br>
<br>
But doesn't <a href="https://reviews.llvm.org/rL299921" rel="noreferrer" target="_blank">https://reviews.llvm.org/rL299921</a> introduce testcases for<br>
arm, thumb, armeb and thumbeb targets to clang driver?</blockquote><div><br>Right, but none of the checks were being done - it was just always true.<br><br>The test coverage that's missing is something that would cause the 'if' on line 877 in that patch to return false. So something that isn't x86_64, isn't mips64, isn't aarch64, and isn't arm (so, maybe PPC? for example). That test would demonstrate that lsan is still enabled in that case when it shouldn't be, because the 'IsArmArch' is always true.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Or perhaps I'm<br>
missing something?<br>
<br>
-Maxim<br>
<br>
><br>
> On Tue, Apr 11, 2017 at 12:34 AM Maxim Ostapenko via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a> <mailto:<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>> wrote:<br>
><br>
>     Author: chefmax<br>
>     Date: Tue Apr 11 02:22:11 2017<br>
>     New Revision: 299921<br>
><br>
>     URL: <a href="http://llvm.org/viewvc/llvm-project?rev=299921&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=299921&view=rev</a><br>
>     Log:<br>
>     [lsan] Enable LSan on arm Linux, clang part<br>
><br>
>     This is a compiler part of <a href="https://reviews.llvm.org/D29586" rel="noreferrer" target="_blank">https://reviews.llvm.org/D29586</a>. Enable<br>
>     LSan on arm Linux.<br>
><br>
>     Differential Revision: <a href="https://reviews.llvm.org/D31760" rel="noreferrer" target="_blank">https://reviews.llvm.org/D31760</a><br>
><br>
>     Modified:<br>
>         cfe/trunk/lib/Driver/ToolChains/Linux.cpp<br>
>         cfe/trunk/test/Driver/fsanitize.c<br>
><br>
>     Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp<br>
>     URL:<br>
>     <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=299921&r1=299920&r2=299921&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=299921&r1=299920&r2=299921&view=diff</a><br>
>     ==============================================================================<br>
>     --- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)<br>
>     +++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Apr 11 02:22:11 2017<br>
>     @@ -864,6 +864,9 @@ SanitizerMask Linux::getSupportedSanitiz<br>
>                                 getTriple().getArch() ==<br>
>     llvm::Triple::ppc64le;<br>
>        const bool IsAArch64 = getTriple().getArch() ==<br>
>     llvm::Triple::aarch64 ||<br>
>                               getTriple().getArch() ==<br>
>     llvm::Triple::aarch64_be;<br>
>     +  const bool IsArmArch = getTriple().getArch() ==<br>
>     llvm::Triple::arm ||<br>
>     +                         llvm::Triple::thumb ||<br>
>     llvm::Triple::armeb ||<br>
>     +                         llvm::Triple::thumbeb;<br>
>        SanitizerMask Res = ToolChain::getSupportedSanitizers();<br>
>        Res |= SanitizerKind::Address;<br>
>        Res |= SanitizerKind::KernelAddress;<br>
>     @@ -871,7 +874,7 @@ SanitizerMask Linux::getSupportedSanitiz<br>
>        Res |= SanitizerKind::SafeStack;<br>
>        if (IsX86_64 || IsMIPS64 || IsAArch64)<br>
>          Res |= SanitizerKind::DataFlow;<br>
>     -  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86)<br>
>     +  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch)<br>
>          Res |= SanitizerKind::Leak;<br>
>        if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64)<br>
>          Res |= SanitizerKind::Thread;<br>
><br>
>     Modified: cfe/trunk/test/Driver/fsanitize.c<br>
>     URL:<br>
>     <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=299921&r1=299920&r2=299921&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=299921&r1=299920&r2=299921&view=diff</a><br>
>     ==============================================================================<br>
>     --- cfe/trunk/test/Driver/fsanitize.c (original)<br>
>     +++ cfe/trunk/test/Driver/fsanitize.c Tue Apr 11 02:22:11 2017<br>
>     @@ -237,6 +237,30 @@<br>
>      // RUN: %clang -target i686-linux-gnu -fsanitize=address,leak<br>
>     -fno-sanitize=address %s -### 2>&1 | FileCheck %s<br>
>     --check-prefix=CHECK-SANA-SANL-NO-SANA-X86<br>
>      // CHECK-SANA-SANL-NO-SANA-X86: "-fsanitize=leak"<br>
><br>
>     +// RUN: %clang -target arm-linux-gnu -fsanitize=leak %s -### 2>&1<br>
>     | FileCheck %s --check-prefix=CHECK-SANL-ARM<br>
>     +// CHECK-SANL-ARM: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target arm-linux-gnu -fsanitize=address,leak<br>
>     -fno-sanitize=address %s -### 2>&1 | FileCheck %s<br>
>     --check-prefix=CHECK-SANA-SANL-NO-SANA-ARM<br>
>     +// CHECK-SANA-SANL-NO-SANA-ARM: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target thumb-linux -fsanitize=leak %s -### 2>&1 |<br>
>     FileCheck %s --check-prefix=CHECK-SANL-THUMB<br>
>     +// CHECK-SANL-THUMB: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target thumb-linux -fsanitize=address,leak<br>
>     -fno-sanitize=address %s -### 2>&1 | FileCheck %s<br>
>     --check-prefix=CHECK-SANA-SANL-NO-SANA-THUMB<br>
>     +// CHECK-SANA-SANL-NO-SANA-THUMB: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target armeb-linux-gnu -fsanitize=leak %s -###<br>
>     2>&1 | FileCheck %s --check-prefix=CHECK-SANL-ARMEB<br>
>     +// CHECK-SANL-ARMEB: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target armeb-linux-gnu -fsanitize=address,leak<br>
>     -fno-sanitize=address %s -### 2>&1 | FileCheck %s<br>
>     --check-prefix=CHECK-SANA-SANL-NO-SANA-ARMEB<br>
>     +// CHECK-SANA-SANL-NO-SANA-ARMEB: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target thumbeb-linux -fsanitize=leak %s -### 2>&1<br>
>     | FileCheck %s --check-prefix=CHECK-SANL-THUMBEB<br>
>     +// CHECK-SANL-THUMBEB: "-fsanitize=leak"<br>
>     +<br>
>     +// RUN: %clang -target thumbeb-linux -fsanitize=address,leak<br>
>     -fno-sanitize=address %s -### 2>&1 | FileCheck %s<br>
>     --check-prefix=CHECK-SANA-SANL-NO-SANA-THUMBEB<br>
>     +// CHECK-SANA-SANL-NO-SANA-THUMBEB: "-fsanitize=leak"<br>
>     +<br>
>      // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -###<br>
>     2>&1 | FileCheck %s --check-prefix=CHECK-MSAN<br>
>      // CHECK-MSAN: "-fno-assume-sane-operator-new"<br>
>      // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s<br>
>     -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN<br>
><br>
><br>
>     _______________________________________________<br>
>     cfe-commits mailing list<br>
>     <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a> <mailto:<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>><br>
>     <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
><br>
<br>
</blockquote></div></div>