<div dir="ltr"><div><div>The tests from <a href="https://reviews.llvm.org/rL299921" rel="noreferrer" style="font-size:12.8px" target="_blank">https://reviews.llvm.org/<wbr>rL299921</a> are not obviously enough. Those tests test only positive cases, thus do not catch when some flag is always true, like the bug we are discussing. There are missing tests that cover negative cases, when some sanitizer is not specified for a target.<br><br></div>Thanks<br><br></div>Galina<br><div><div><div><br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 5, 2017 at 10:28 AM, Maxim Ostapenko via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 05/06/17 20:16, David Blaikie wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This change seemed to be buggy (& I assume missing test coverage to demonstrate that). Galina fixed it in <a href="http://llvm.org/viewvc/llvm-project?rev=304663&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=304663&view=rev</a> based on a compiler warning.<br>
</blockquote>
<br></span>
Oh, right, I've missed that. Sorry!<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Can you add test coverage to this code to exercise these untested codepaths?<br>
</blockquote>
<br></span>
But doesn't <a href="https://reviews.llvm.org/rL299921" rel="noreferrer" target="_blank">https://reviews.llvm.org/rL299<wbr>921</a> introduce testcases for arm, thumb, armeb and thumbeb targets to clang driver? Or perhaps I'm missing something?<br>
<br>
-Maxim<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
On Tue, Apr 11, 2017 at 12:34 AM Maxim Ostapenko via cfe-commits <<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<wbr>.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-pr<wbr>oject?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/D2958<wbr>6</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/D3176<wbr>0</a><br>
<br>
    Modified:<br>
        cfe/trunk/lib/Driver/ToolChain<wbr>s/Linux.cpp<br>
        cfe/trunk/test/Driver/fsanitiz<wbr>e.c<br>
<br>
    Modified: cfe/trunk/lib/Driver/ToolChain<wbr>s/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-pr<wbr>oject/cfe/trunk/lib/Driver/Too<wbr>lChains/Linux.cpp?rev=299921&<wbr>r1=299920&r2=299921&view=diff</a><br>
    ==============================<wbr>==============================<wbr>==================<br>
    --- cfe/trunk/lib/Driver/ToolChain<wbr>s/Linux.cpp (original)<br>
    +++ cfe/trunk/lib/Driver/ToolChain<wbr>s/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::getSupportedSanitiz<wbr>ers();<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/fsanitiz<wbr>e.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-pr<wbr>oject/cfe/trunk/test/Driver/fs<wbr>anitize.c?rev=299921&r1=299920<wbr>&r2=299921&view=diff</a><br>
    ==============================<wbr>==============================<wbr>==================<br>
    --- cfe/trunk/test/Driver/fsanitiz<wbr>e.c (original)<br>
    +++ cfe/trunk/test/Driver/fsanitiz<wbr>e.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<wbr>-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<wbr>-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-THUM<wbr>B<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<wbr>-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-ARME<wbr>B<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<wbr>-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-THUM<wbr>BEB<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<wbr>-NO-SANA-THUMBEB<br>
    +// CHECK-SANA-SANL-NO-SANA-THUMBE<wbr>B: "-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<wbr>"<br>
     // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s<br>
    -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN<br>
<br>
<br>
    ______________________________<wbr>_________________<br>
    cfe-commits mailing list<br></div></div>
    <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<wbr>.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/<wbr>mailman/listinfo/cfe-commits</a><br>
<br>
</blockquote><div class="HOEnZb"><div class="h5">
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<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/<wbr>mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>