<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">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">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><br></div><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">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">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">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">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">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">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">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">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">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">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">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">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>
                        </blockquote>
                      </div>
                      <br>
                      _______________________________________________<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/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">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>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </div>

_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>