<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 7/26/18 1:38 PM, Richard Smith
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAOfiQqmzKPwPoB7gKUbEefpykM-sBaN5eqBj4xD9fHDBZjNYzA@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      <div dir="ltr">
        <div class="gmail_quote">
          <div dir="ltr">On Thu, 26 Jul 2018 at 13:14, Erik Pilkington
            via cfe-commits <<a
              href="mailto:cfe-commits@lists.llvm.org"
              moz-do-not-send="true">cfe-commits@lists.llvm.org</a>>
            wrote:<br>
          </div>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div text="#000000" bgcolor="#FFFFFF">
              <p><br>
              </p>
              <br>
              <div class="m_-7979429682305634951moz-cite-prefix">On
                7/26/18 12:52 PM, Richard Smith wrote:<br>
              </div>
              <blockquote type="cite">
                <div dir="ltr">Other than the (fixed) ffmpeg false
                  positive, I see this firing in three places.
                  <div><br>
                  </div>
                  <div>One of them is in the libc tests for memset and
                    bzero, where the 0 argument is intentional.</div>
                </div>
              </blockquote>
              <br>
              I don't find this case very convincing, if you're
              literally the one person that has to test memset then you
              should probably just compile with
              -Wno-suspicious-memaccess.<br>
            </div>
          </blockquote>
          <div><br>
          </div>
          <div>I think you're missing the point. In our tests, the false
            positive rate for this was extremely high. If the warning
            should be turned off in two-thirds of the places where it
            appears, then it should not be on by default. That's why I
            was asking for evidence that it's actually got a much higher
            true-positive rate than we saw.</div>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div text="#000000" bgcolor="#FFFFFF">
              <blockquote type="cite">
                <div dir="ltr">
                  <div>One of them is here: <a
href="https://github.com/apache/xerces-c/blob/trunk/src/xercesc/util/XMLUTF16Transcoder.cpp#L114"
                      target="_blank" moz-do-not-send="true">https://github.com/apache/xerces-c/blob/trunk/src/xercesc/util/XMLUTF16Transcoder.cpp#L114</a>
                    (note that this code is correct).</div>
                  <div>And one of them was a real bug where the second
                    and third arguments of memset were transposed.</div>
                  <div><br>
                  </div>
                  <div>That's an extremely low false positive rate, much
                    lower than what we'd expect for an
                    enabled-by-default warning. I find this extremely
                    surprising; I would have expected the ratio of
                    true-- to false-positives to be much much higher.
                    But ultimately data wins out over expectations.</div>
                  <div>How does our experience compare to other people's
                    experiences? Are our findings abnormal? (They may
                    well be; our corpus is very C++-heavy.) If this is
                    indeed typical, we should reconsider whether to have
                    these warnings enabled by default, as they do not
                    meet our bar for false positives.</div>
                </div>
              </blockquote>
              <br>
              I tested this internally, and it found ~50 true-positives
              and only one false-positive (similar to apache bug above,
              where someone was memsetting to a sizeof exression). Given
              those numbers, my position is to keep it on-by-default,
              but I'm open to hear more. We might be able to make this
              warning even more conservative in this case by checking if
              the sizeof expression is actually computing a size that
              corresponds to the type of the third argument. I think
              that would be a good tradeoff if it meant we could keep
              this on by default.<br>
            </div>
          </blockquote>
          <div><br>
          </div>
          <div>50:1 sounds a lot more reasonable than what I was seeing
            (1:3). In the past we've said >99% true-positive was a
            good threshold for on-by-default warnings; your data is a
            plausible sample set from such a Bernoulli distribution
            (whereas mine is ... not so much -- but I think this is at
            least evidence that in my corpus, something else had already
            caught most of these bugs already; do we have a pre-existing
            ClangTidy check for this or similar? Does GCC warn on it?).<br>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    Yep, GCC has -Wmemset-transposed-args already, and there is also a
    clang-tidy check. If your codebase already has to pass through those
    tools then I agree thats probably the reason your seeing those
    numbers. <br>
    <br>
    <blockquote type="cite"
cite="mid:CAOfiQqmzKPwPoB7gKUbEefpykM-sBaN5eqBj4xD9fHDBZjNYzA@mail.gmail.com">
      <div dir="ltr">
        <div class="gmail_quote">
          <div>If we can improve the true-positive rate further, that'd
            be a nice-to-have, but given the sizes of our respective
            sample sets, I think we lack the data to know if we've
            really helped the true-positive rate very much.</div>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div text="#000000" bgcolor="#FFFFFF">
              <blockquote type="cite">
                <div class="gmail_quote">
                  <div dir="ltr">On Mon, 23 Jul 2018 at 09:28, Erik
                    Pilkington via cfe-commits <<a
                      href="mailto:cfe-commits@lists.llvm.org"
                      target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a>>
                    wrote:<br>
                  </div>
                  <blockquote class="gmail_quote" style="margin:0 0 0
                    .8ex;border-left:1px #ccc solid;padding-left:1ex">
                    <div text="#000000" bgcolor="#FFFFFF">
                      <p>Sure, that seems pretty reasonable. r337706.
                        Thanks for the suggestion!</p>
                      <br>
                      <div
                        class="m_-7979429682305634951m_8770740850700139232moz-cite-prefix">On
                        7/21/18 1:40 PM, Arthur O'Dwyer wrote:<br>
                      </div>
                      <blockquote type="cite">
                        <div dir="ltr">
                          <div>In this case Clang is complaining about</div>
                          <div><br>
                          </div>
                          <div>    memset(complicated-expr, 0, 0);</div>
                          <div><br>
                          </div>
                          <div>which means that transposing the last two
                            arguments would not "fix" anything. Clang
                            ought to not complain in this case.</div>
                          <div>It doesn't have anything to do with
                            coming from a macro; it's just that <i>both</i>
                            arguments are zero.</div>
                          <div>(I would also expect that
                            `memset(complicated-expr, (unsigned
                            char)fill, (size_t)0)` shouldn't warn,
                            either. But we had some disagreement that
                            casts to `(unsigned char)` resp. `(size_t)`
                            should be the right way to shut up the
                            warning in the first place...)</div>
                          <div>Basically Clang should only be suggesting
                            a swap (and thus complaining at all) in the
                            case that swapping the arguments would
                            actually improve the situation.</div>
                          <div><br>
                          </div>
                          <div>–Arthur</div>
                          <div><br>
                          </div>
                          On Sat, Jul 21, 2018 at 1:33 PM, Nico Weber
                          via cfe-commits <span dir="ltr"><<a
                              href="mailto:cfe-commits@lists.llvm.org"
                              target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a>></span>
                          wrote:<br>
                          <div class="gmail_extra">
                            <div class="gmail_quote">
                              <blockquote class="gmail_quote"
                                style="margin:0 0 0 .8ex;border-left:1px
                                #ccc solid;padding-left:1ex">
                                <div dir="ltr">This has a false positive
                                  on ffmpeg:<br>
                                  <div><br>
                                  </div>
                                  <div>
                                    <div>../../third_party/ffmpeg/libavcodec/options.c:273:64:
                                      error: 'size' argument to memset
                                      is '0'; did you mean to transpose
                                      the last two arguments?
                                      [-Werror,-Wmemset-transposed-args]</div>
                                    <div>alloc_and_copy_or_fail(intra_matrix,
                                      64 * sizeof(int16_t), 0);</div>
                                  </div>
                                  <div><br>
                                  </div>
                                  <div>With:</div>
                                  <div><br>
                                  </div>
                                  <div>
                                    <div>#define
                                      alloc_and_copy_or_fail(obj, size,
                                      pad) \</div>
                                    <div>    if (src->obj &&
                                      size > 0) { \</div>
                                    <div>        dest->obj =
                                      av_malloc(size + pad); \</div>
                                    <div>        if (!dest->obj) \</div>
                                    <div>            goto fail; \</div>
                                    <div>        memcpy(dest->obj,
                                      src->obj, size); \</div>
                                    <div>        if (pad) \</div>
                                    <div>            memset(((uint8_t *)
                                      dest->obj) + size, 0, pad); \</div>
                                    <div>    }</div>
                                  </div>
                                  <div><br>
                                  </div>
                                  <div>(<a
href="https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavcodec/options.c?q=alloc_and_copy_or_fail&sq=package:chromium&g=0&l=261"
                                      target="_blank"
                                      moz-do-not-send="true">https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavcodec/options.c?q=alloc_and_copy_or_fail&sq=package:chromium&g=0&l=261</a>
                                    ; <a
                                      href="https://bugs.chromium.org/p/chromium/issues/detail?id=866202"
                                      target="_blank"
                                      moz-do-not-send="true">https://bugs.chromium.org/p/chromium/issues/detail?id=866202</a>)</div>
                                  <div><br>
                                  </div>
                                  <div>Maybe the warning shouldn't fire
                                    if the memset is from a macro?</div>
                                </div>
                                <br>
                                <div class="gmail_quote">
                                  <div dir="ltr">On Thu, Jul 19, 2018 at
                                    12:51 PM Erik Pilkington via
                                    cfe-commits <<a
                                      href="mailto:cfe-commits@lists.llvm.org"
                                      target="_blank"
                                      moz-do-not-send="true">cfe-commits@lists.llvm.org</a>>
                                    wrote:<br>
                                  </div>
                                  <blockquote class="gmail_quote"
                                    style="margin:0 0 0
                                    .8ex;border-left:1px #ccc
                                    solid;padding-left:1ex">Author:
                                    epilk<br>
                                    Date: Thu Jul 19 09:46:15 2018<br>
                                    New Revision: 337470<br>
                                    <br>
                                    URL: <a
                                      href="http://llvm.org/viewvc/llvm-project?rev=337470&view=rev"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">http://llvm.org/viewvc/llvm-project?rev=337470&view=rev</a><br>
                                    Log:<br>
                                    [Sema] Add a new warning,
                                    -Wmemset-transposed-args<br>
                                    <br>
                                    This diagnoses calls to memset that
                                    have the second and third arguments<br>
                                    transposed, for example:<br>
                                    <br>
                                      memset(buf, sizeof(buf), 0);<br>
                                    <br>
                                    This is done by checking if the
                                    third argument is a literal 0, or if
                                    the second<br>
                                    is a sizeof expression (and the
                                    third isn't). The first check is
                                    also done for<br>
                                    calls to bzero.<br>
                                    <br>
                                    Differential revision: <a
                                      href="https://reviews.llvm.org/D49112"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">https://reviews.llvm.org/D49112</a><br>
                                    <br>
                                    Added:<br>
                                       
                                    cfe/trunk/test/Sema/transpose-memset.c<br>
                                    Modified:<br>
                                       
                                    cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
                                       
                                    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
                                       
                                    cfe/trunk/lib/Sema/SemaChecking.cpp<br>
                                    <br>
                                    Modified:
                                    cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
                                    URL: <a
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=337470&r1=337469&r2=337470&view=diff"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=337470&r1=337469&r2=337470&view=diff</a><br>
==============================================================================<br>
                                    ---
                                    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
                                    (original)<br>
                                    +++
                                    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
                                    Thu Jul 19 09:46:15 2018<br>
                                    @@ -443,6 +443,13 @@ def :
                                    DiagGroup<"synth">;<br>
                                     def SizeofArrayArgument :
                                    DiagGroup<"sizeof-array-argument">;<br>
                                     def SizeofArrayDecay :
                                    DiagGroup<"sizeof-array-decay">;<br>
                                     def SizeofPointerMemaccess :
                                    DiagGroup<"sizeof-pointer-memaccess">;<br>
                                    +def MemsetTransposedArgs :
                                    DiagGroup<"memset-transposed-args">;<br>
                                    +def DynamicClassMemaccess :
                                    DiagGroup<"dynamic-class-memaccess">;<br>
                                    +def NonTrivialMemaccess :
                                    DiagGroup<"nontrivial-memaccess">;<br>
                                    +def SuspiciousBzero :
                                    DiagGroup<"suspicious-bzero">;<br>
                                    +def SuspiciousMemaccess :
                                    DiagGroup<"suspicious-memaccess",<br>
                                    +  [SizeofPointerMemaccess,
                                    DynamicClassMemaccess,<br>
                                    +   NonTrivialMemaccess,
                                    MemsetTransposedArgs,
                                    SuspiciousBzero]>;<br>
                                     def StaticInInline :
                                    DiagGroup<"static-in-inline">;<br>
                                     def StaticLocalInInline :
                                    DiagGroup<"static-local-in-inline">;<br>
                                     def GNUStaticFloatInit :
                                    DiagGroup<"gnu-static-float-init">;<br>
                                    <br>
                                    Modified:
                                    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
                                    URL: <a
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=337470&r1=337469&r2=337470&view=diff"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=337470&r1=337469&r2=337470&view=diff</a><br>
==============================================================================<br>
                                    ---
                                    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
                                    (original)<br>
                                    +++
                                    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
                                    Thu Jul 19 09:46:15 2018<br>
                                    @@ -619,14 +619,14 @@ def
                                    warn_cstruct_memaccess : Warning<<br>
                                       "%select{destination for|source
                                    of|first operand of|second operand
                                    of}0 this "<br>
                                       "%1 call is a pointer to record
                                    %2 that is not trivial to "<br>
                                     
                                     "%select{primitive-default-initialize|primitive-copy}3">,<br>
                                    - 
                                    InGroup<DiagGroup<"nontrivial-memaccess">>;<br>
                                    + 
                                    InGroup<NonTrivialMemaccess>;<br>
                                     def note_nontrivial_field :
                                    Note<<br>
                                       "field is non-trivial to
                                    %select{copy|default-initialize}0">;<br>
                                     def warn_dyn_class_memaccess :
                                    Warning<<br>
                                       "%select{destination for|source
                                    of|first operand of|second operand
                                    of}0 this "<br>
                                       "%1 call is a pointer to
                                    %select{|class containing a
                                    }2dynamic class %3; "<br>
                                       "vtable pointer will be
                                    %select{overwritten|copied|moved|compared}4">,<br>
                                    - 
                                    InGroup<DiagGroup<"dynamic-class-memaccess">>;<br>
                                    + 
                                    InGroup<DynamicClassMemaccess>;<br>
                                     def note_bad_memaccess_silence :
                                    Note<<br>
                                       "explicitly cast the pointer to
                                    silence this warning">;<br>
                                     def
                                    warn_sizeof_pointer_expr_memaccess :
                                    Warning<<br>
                                    @@ -655,7 +655,19 @@ def
                                    note_memsize_comparison_paren : Note<br>
                                       "did you mean to compare the
                                    result of %0 instead?">;<br>
                                     def
                                    note_memsize_comparison_cast_silence
                                    : Note<<br>
                                       "explicitly cast the argument to
                                    size_t to silence this warning">;<br>
                                    -  <br>
                                    +def warn_suspicious_sizeof_memset :
                                    Warning<<br>
                                    +  "%select{'size' argument to
                                    memset is '0'|"<br>
                                    +  "setting buffer to a 'sizeof'
                                    expression}0"<br>
                                    +  "; did you mean to transpose the
                                    last two arguments?">,<br>
                                    + 
                                    InGroup<MemsetTransposedArgs>;<br>
                                    +def
                                    note_suspicious_sizeof_memset_silence
                                    : Note<<br>
                                    +  "%select{parenthesize the third
                                    argument|"<br>
                                    +  "cast the second argument to
                                    'int'}0 to silence">;<br>
                                    +def warn_suspicious_bzero_size :
                                    Warning<"'size' argument to bzero
                                    is '0'">,<br>
                                    +  InGroup<SuspiciousBzero>;<br>
                                    +def
                                    note_suspicious_bzero_size_silence :
                                    Note<<br>
                                    +  "parenthesize the second argument
                                    to silence">;<br>
                                    +<br>
                                     def warn_strncat_large_size :
                                    Warning<<br>
                                       "the value of the size argument
                                    in 'strncat' is too large, might
                                    lead to a " <br>
                                       "buffer overflow">,
                                    InGroup<StrncatSize>;<br>
                                    <br>
                                    Modified:
                                    cfe/trunk/lib/Sema/SemaChecking.cpp<br>
                                    URL: <a
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=337470&r1=337469&r2=337470&view=diff"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=337470&r1=337469&r2=337470&view=diff</a><br>
==============================================================================<br>
                                    ---
                                    cfe/trunk/lib/Sema/SemaChecking.cpp
                                    (original)<br>
                                    +++
                                    cfe/trunk/lib/Sema/SemaChecking.cpp
                                    Thu Jul 19 09:46:15 2018<br>
                                    @@ -8629,24 +8629,26 @@ static const
                                    CXXRecordDecl *getContained<br>
                                       return nullptr;<br>
                                     }<br>
                                    <br>
                                    +static const
                                    UnaryExprOrTypeTraitExpr
                                    *getAsSizeOfExpr(const Expr *E) {<br>
                                    +  if (const auto *Unary =
                                    dyn_cast<UnaryExprOrTypeTraitExpr>(E))<br>
                                    +    if (Unary->getKind() ==
                                    UETT_SizeOf)<br>
                                    +      return Unary;<br>
                                    +  return nullptr;<br>
                                    +}<br>
                                    +<br>
                                     /// If E is a sizeof expression,
                                    returns its argument expression,<br>
                                     /// otherwise returns NULL.<br>
                                     static const Expr
                                    *getSizeOfExprArg(const Expr *E) {<br>
                                    -  if (const
                                    UnaryExprOrTypeTraitExpr *SizeOf =<br>
                                    -     
                                    dyn_cast<UnaryExprOrTypeTraitExpr>(E))<br>
                                    -    if (SizeOf->getKind() ==
                                    UETT_SizeOf &&
                                    !SizeOf->isArgumentType())<br>
                                    +  if (const
                                    UnaryExprOrTypeTraitExpr *SizeOf =
                                    getAsSizeOfExpr(E))<br>
                                    +    if
                                    (!SizeOf->isArgumentType())<br>
                                           return
                                    SizeOf->getArgumentExpr()->IgnoreParenImpCasts();<br>
                                    -<br>
                                       return nullptr;<br>
                                     }<br>
                                    <br>
                                     /// If E is a sizeof expression,
                                    returns its argument type.<br>
                                     static QualType
                                    getSizeOfArgType(const Expr *E) {<br>
                                    -  if (const
                                    UnaryExprOrTypeTraitExpr *SizeOf =<br>
                                    -     
                                    dyn_cast<UnaryExprOrTypeTraitExpr>(E))<br>
                                    -    if (SizeOf->getKind() ==
                                    UETT_SizeOf)<br>
                                    -      return
                                    SizeOf->getTypeOfArgument();<br>
                                    -<br>
                                    +  if (const
                                    UnaryExprOrTypeTraitExpr *SizeOf =
                                    getAsSizeOfExpr(E))<br>
                                    +    return
                                    SizeOf->getTypeOfArgument();<br>
                                       return QualType();<br>
                                     }<br>
                                    <br>
                                    @@ -8742,6 +8744,86 @@ struct
                                    SearchNonTrivialToCopyField<br>
                                    <br>
                                     }<br>
                                    <br>
                                    +/// Detect if \c SizeofExpr is
                                    likely to calculate the sizeof an
                                    object.<br>
                                    +static bool
                                    doesExprLikelyComputeSize(const Expr
                                    *SizeofExpr) {<br>
                                    +  SizeofExpr =
                                    SizeofExpr->IgnoreParenImpCasts();<br>
                                    +<br>
                                    +  if (const auto *BO =
                                    dyn_cast<BinaryOperator>(SizeofExpr))
                                    {<br>
                                    +    if (BO->getOpcode() !=
                                    BO_Mul && BO->getOpcode()
                                    != BO_Add)<br>
                                    +      return false;<br>
                                    +<br>
                                    +    return
                                    doesExprLikelyComputeSize(BO->getLHS())
                                    ||<br>
                                    +         
                                     doesExprLikelyComputeSize(BO->getRHS());<br>
                                    +  }<br>
                                    +<br>
                                    +  return
                                    getAsSizeOfExpr(SizeofExpr) !=
                                    nullptr;<br>
                                    +}<br>
                                    +<br>
                                    +/// Check if the ArgLoc originated
                                    from a macro passed to the call at
                                    CallLoc.<br>
                                    +///<br>
                                    +/// \code<br>
                                    +///   #define MACRO 0<br>
                                    +///   foo(MACRO);<br>
                                    +///   foo(0);<br>
                                    +/// \endcode<br>
                                    +///<br>
                                    +/// This should return true for the
                                    first call to foo, but not for the
                                    second<br>
                                    +/// (regardless of whether foo is a
                                    macro or function).<br>
                                    +static bool
                                    isArgumentExpandedFromMacro(SourceManager
                                    &SM,<br>
                                    +                                   
                                        SourceLocation CallLoc,<br>
                                    +                                   
                                        SourceLocation ArgLoc) {<br>
                                    +  if (!CallLoc.isMacroID())<br>
                                    +    return SM.getFileID(CallLoc) !=
                                    SM.getFileID(ArgLoc);<br>
                                    +<br>
                                    +  return
                                    SM.getFileID(SM.getImmediateMacroCallerLoc(CallLoc))
                                    !=<br>
                                    +       
                                     SM.getFileID(SM.getImmediateMacroCallerLoc(ArgLoc));<br>
                                    +}<br>
                                    +<br>
                                    +/// Diagnose cases like
                                    'memset(buf, sizeof(buf), 0)', which
                                    should have the<br>
                                    +/// last two arguments transposed.<br>
                                    +static void CheckMemaccessSize(Sema
                                    &S, unsigned BId, const CallExpr
                                    *Call) {<br>
                                    +  if (BId != Builtin::BImemset
                                    && BId != Builtin::BIbzero)<br>
                                    +    return;<br>
                                    +<br>
                                    +  const Expr *SizeArg =<br>
                                    +    Call->getArg(BId ==
                                    Builtin::BImemset ? 2 :
                                    1)->IgnoreImpCasts();<br>
                                    +<br>
                                    +  // If we're memsetting or
                                    bzeroing 0 bytes, then this is
                                    likely an error.<br>
                                    +  SourceLocation CallLoc =
                                    Call->getRParenLoc();<br>
                                    +  SourceManager &SM =
                                    S.getSourceManager();<br>
                                    +  if
                                    (isa<IntegerLiteral>(SizeArg)
                                    &&<br>
                                    +     
                                    cast<IntegerLiteral>(SizeArg)->getValue()
                                    == 0 &&<br>
                                    +     
                                    !isArgumentExpandedFromMacro(SM,
                                    CallLoc, SizeArg->getExprLoc()))
                                    {<br>
                                    +<br>
                                    +    SourceLocation DiagLoc =
                                    SizeArg->getExprLoc();<br>
                                    +<br>
                                    +    // Some platforms #define bzero
                                    to __builtin_memset. See if this is
                                    the<br>
                                    +    // case, and if so, emit a
                                    better diagnostic.<br>
                                    +    if (BId == Builtin::BIbzero ||<br>
                                    +        (CallLoc.isMacroID()
                                    &&
                                    Lexer::getImmediateMacroName(<br>
                                    +                                   
                                    CallLoc, SM, S.getLangOpts()) ==
                                    "bzero")) {<br>
                                    +      S.Diag(DiagLoc,
                                    diag::warn_suspicious_bzero_size);<br>
                                    +      S.Diag(DiagLoc,
                                    diag::note_suspicious_bzero_size_silence);<br>
                                    +    } else {<br>
                                    +      S.Diag(DiagLoc,
                                    diag::warn_suspicious_sizeof_memset)
                                    << 0;<br>
                                    +      S.Diag(DiagLoc,
                                    diag::note_suspicious_sizeof_memset_silence)
                                    << 0;<br>
                                    +    }<br>
                                    +    return;<br>
                                    +  }<br>
                                    +<br>
                                    +  // If the second argument to a
                                    memset is a sizeof expression and
                                    the third<br>
                                    +  // isn't, this is also likely an
                                    error. This should catch<br>
                                    +  // 'memset(buf, sizeof(buf),
                                    0xff)'.<br>
                                    +  if (BId == Builtin::BImemset
                                    &&<br>
                                    +     
                                    doesExprLikelyComputeSize(Call->getArg(1))
                                    &&<br>
                                    +     
                                    !doesExprLikelyComputeSize(Call->getArg(2)))
                                    {<br>
                                    +    SourceLocation DiagLoc =
                                    Call->getArg(1)->getExprLoc();<br>
                                    +    S.Diag(DiagLoc,
                                    diag::warn_suspicious_sizeof_memset)
                                    << 1;<br>
                                    +    S.Diag(DiagLoc,
                                    diag::note_suspicious_sizeof_memset_silence)
                                    << 1;<br>
                                    +    return;<br>
                                    +  }<br>
                                    +}<br>
                                    +<br>
                                     /// Check for dangerous or invalid
                                    arguments to memset().<br>
                                     ///<br>
                                     /// This issues warnings on known
                                    problematic, dangerous or
                                    unspecified<br>
                                    @@ -8771,6 +8853,9 @@ void
                                    Sema::CheckMemaccessArguments(const<br>
                                                                       
                                      Call->getLocStart(),
                                    Call->getRParenLoc()))<br>
                                         return;<br>
                                    <br>
                                    +  // Catch cases like 'memset(buf,
                                    sizeof(buf), 0)'.<br>
                                    +  CheckMemaccessSize(*this, BId,
                                    Call);<br>
                                    +<br>
                                       // We have special checking when
                                    the length is a sizeof expression.<br>
                                       QualType SizeOfArgTy =
                                    getSizeOfArgType(LenExpr);<br>
                                       const Expr *SizeOfArg =
                                    getSizeOfExprArg(LenExpr);<br>
                                    <br>
                                    Added:
                                    cfe/trunk/test/Sema/transpose-memset.c<br>
                                    URL: <a
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/transpose-memset.c?rev=337470&view=auto"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/transpose-memset.c?rev=337470&view=auto</a><br>
==============================================================================<br>
                                    ---
                                    cfe/trunk/test/Sema/transpose-memset.c
                                    (added)<br>
                                    +++
                                    cfe/trunk/test/Sema/transpose-memset.c
                                    Thu Jul 19 09:46:15 2018<br>
                                    @@ -0,0 +1,60 @@<br>
                                    +// RUN: %clang_cc1     
                                     -Wmemset-transposed-args -verify %s<br>
                                    +// RUN: %clang_cc1 -xc++
                                    -Wmemset-transposed-args -verify %s<br>
                                    +<br>
                                    +#define memset(...)
                                    __builtin_memset(__VA_ARGS__)<br>
                                    +#define bzero(x,y)
                                    __builtin_memset(x, 0, y)<br>
                                    +#define real_bzero(x,y)
                                    __builtin_bzero(x,y)<br>
                                    +<br>
                                    +int array[10];<br>
                                    +int *ptr;<br>
                                    +<br>
                                    +int main() {<br>
                                    +  memset(array, sizeof(array), 0);
                                    // expected-warning{{'size' argument
                                    to memset is '0'; did you mean to
                                    transpose the last two arguments?}}
                                    expected-note{{parenthesize the
                                    third argument to silence}}<br>
                                    +  memset(array, sizeof(array),
                                    0xff); // expected-warning{{setting
                                    buffer to a 'sizeof' expression; did
                                    you mean to transpose the last two
                                    arguments?}} expected-note{{cast the
                                    second argument to 'int' to
                                    silence}}<br>
                                    +  memset(ptr, sizeof(ptr), 0); //
                                    expected-warning{{'size' argument to
                                    memset is '0'; did you mean to
                                    transpose the last two arguments?}}
                                    expected-note{{parenthesize the
                                    third argument to silence}}<br>
                                    +  memset(ptr, sizeof(*ptr) * 10,
                                    1); // expected-warning{{setting
                                    buffer to a 'sizeof' expression; did
                                    you mean to transpose the last two
                                    arguments?}} expected-note{{cast the
                                    second argument to 'int' to
                                    silence}}<br>
                                    +  memset(ptr, 10 * sizeof(int *),
                                    1); // expected-warning{{setting
                                    buffer to a 'sizeof' expression; did
                                    you mean to transpose the last two
                                    arguments?}} expected-note{{cast the
                                    second argument to 'int' to
                                    silence}}<br>
                                    +  memset(ptr, 10 * sizeof(int *) +
                                    10, 0xff); //
                                    expected-warning{{setting buffer to
                                    a 'sizeof' expression; did you mean
                                    to transpose the last two
                                    arguments?}} expected-note{{cast the
                                    second argument to 'int' to
                                    silence}}<br>
                                    +  memset(ptr, sizeof(char) *
                                    sizeof(int *), 0xff); //
                                    expected-warning{{setting buffer to
                                    a 'sizeof' expression; did you mean
                                    to transpose the last two
                                    arguments?}} expected-note{{cast the
                                    second argument to 'int' to
                                    silence}}<br>
                                    +  memset(array, sizeof(array),
                                    sizeof(array)); // Uh... fine I
                                    guess.<br>
                                    +  memset(array, 0, sizeof(array));<br>
                                    +  memset(ptr, 0, sizeof(int *) *
                                    10);<br>
                                    +  memset(array, (int)sizeof(array),
                                    (0)); // no warning<br>
                                    +  memset(array, (int)sizeof(array),
                                    32); // no warning<br>
                                    +  memset(array, 32, (0)); // no
                                    warning<br>
                                    +<br>
                                    +  bzero(ptr, 0); //
                                    expected-warning{{'size' argument to
                                    bzero is '0'}}
                                    expected-note{{parenthesize the
                                    second argument to silence}}<br>
                                    +  real_bzero(ptr, 0); //
                                    expected-warning{{'size' argument to
                                    bzero is '0'}}
                                    expected-note{{parenthesize the
                                    second argument to silence}}<br>
                                    +}<br>
                                    +<br>
                                    +void macros() {<br>
                                    +#define ZERO 0<br>
                                    +  int array[10];<br>
                                    +  memset(array, 0xff, ZERO); // no
                                    warning<br>
                                    +  // Still emit a diagnostic for
                                    memsetting a sizeof expression:<br>
                                    +  memset(array, sizeof(array),
                                    ZERO); //
                                    expected-warning{{'sizeof'}}
                                    expected-note{{cast}}<br>
                                    +  bzero(array, ZERO); // no warning<br>
                                    +  real_bzero(array, ZERO); // no
                                    warning<br>
                                    +#define NESTED_DONT_DIAG           
                                                \<br>
                                    +  memset(array, 0xff, ZERO);       
                                                \<br>
                                    +  real_bzero(array, ZERO);<br>
                                    +<br>
                                    +  NESTED_DONT_DIAG;<br>
                                    +<br>
                                    +#define NESTED_DO_DIAG             
                                                \<br>
                                    +  memset(array, 0xff, 0);         
                                                 \<br>
                                    +  real_bzero(array, 0)<br>
                                    +<br>
                                    +  NESTED_DO_DIAG; //
                                    expected-warning{{'size' argument to
                                    memset}} expected-warning{{'size'
                                    argument to bzero}}
                                    expected-note2{{parenthesize}}<br>
                                    +<br>
                                    +#define FN_MACRO(p)               
                                                 \<br>
                                    +  memset(array, 0xff, p)<br>
                                    +<br>
                                    +  FN_MACRO(ZERO);<br>
                                    +  FN_MACRO(0); // FIXME: should we
                                    diagnose this?<br>
                                    +<br>
                                    +  __builtin_memset(array, 0, ZERO);
                                    // no warning<br>
                                    +  __builtin_bzero(array, ZERO);<br>
                                    +  __builtin_memset(array, 0, 0); //
                                    expected-warning{{'size' argument to
                                    memset}} //
                                    expected-note{{parenthesize}}<br>
                                    +  __builtin_bzero(array, 0); //
                                    expected-warning{{'size' argument to
                                    bzero}} //
                                    expected-note{{parenthesize}}<br>
                                    +}<br>
                                    <br>
                                    <br>
_______________________________________________<br>
                                    cfe-commits mailing list<br>
                                    <a
                                      href="mailto:cfe-commits@lists.llvm.org"
                                      target="_blank"
                                      moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
                                    <a
                                      href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
                                      rel="noreferrer" target="_blank"
                                      moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
                                  </blockquote>
                                </div>
                                <br>
_______________________________________________<br>
                                cfe-commits mailing list<br>
                                <a
                                  href="mailto:cfe-commits@lists.llvm.org"
                                  target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
                                <a
                                  href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
                                  rel="noreferrer" target="_blank"
                                  moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
                                <br>
                              </blockquote>
                            </div>
                            <br>
                          </div>
                        </div>
                      </blockquote>
                      <br>
                    </div>
                    _______________________________________________<br>
                    cfe-commits mailing list<br>
                    <a href="mailto:cfe-commits@lists.llvm.org"
                      target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
                    <a
                      href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
                      rel="noreferrer" target="_blank"
                      moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
                  </blockquote>
                </div>
              </blockquote>
              <br>
            </div>
            _______________________________________________<br>
            cfe-commits mailing list<br>
            <a href="mailto:cfe-commits@lists.llvm.org" target="_blank"
              moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
            <a
              href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
              rel="noreferrer" target="_blank" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
          </blockquote>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>