1. Feb 15, 2022
  2. Feb 14, 2022
    • Momchil Velikov's avatar
      Fix test failure for targets with varying uwtable defaults · a31d00dd
      Momchil Velikov authored
      Depending on toolchain and ABI, a target might not output DWARF unwind tables by default.
      Run the test for a target with a known behaviour, test coverage is not reduced.
      
      Reviewed By: dmgreen
      
      Differential Revision: https://reviews.llvm.org/D119724
      a31d00dd
    • Igor Zhukov's avatar
      [libc++] [NFC] Fix a typo in Cxx2bIssues.csv · 01d8cb3e
      Igor Zhukov authored
      Differential Revision: https://reviews.llvm.org/D119694
      01d8cb3e
    • Balazs Benics's avatar
      [analyzer] Fix taint rule of fgets and setproctitle_init · bf5963bf
      Balazs Benics authored
      There was a typo in the rule.
      `{{0}, ReturnValueIndex}` meant that the discrete index is `0` and the
      variadic index is `-1`.
      What we wanted instead is that both `0` and `-1` are in the discrete index
      list.
      
      Instead of this, we wanted to express that both `0` and the
      `ReturnValueIndex` is in the discrete arg list.
      
      The manual inspection revealed that `setproctitle_init` also suffered a
      probably incomplete propagation rule.
      
      Reviewed By: Szelethus, gamesh411
      
      Differential Revision: https://reviews.llvm.org/D119129
      bf5963bf
    • Balazs Benics's avatar
      [analyzer] Fix taint propagation by remembering to the location context · b099e1e5
      Balazs Benics authored
      Fixes the issue D118987 by mapping the propagation to the callsite's
      LocationContext.
      This way we can keep track of the in-flight propagations.
      
      Note that empty propagation sets won't be inserted.
      
      Reviewed By: NoQ, Szelethus
      
      Differential Revision: https://reviews.llvm.org/D119128
      b099e1e5
    • Balazs Benics's avatar
      [analyzer] Add failing test case demonstrating buggy taint propagation · 744745ae
      Balazs Benics authored
      Recently we uncovered a serious bug in the `GenericTaintChecker`.
      It was already flawed before D116025, but that was the patch that turned
      this silent bug into a crash.
      
      It happens if the `GenericTaintChecker` has a rule for a function, which
      also has a definition.
      
        char *fgets(char *s, int n, FILE *fp) {
          nested_call();   // no parameters!
          return (char *)0;
        }
      
        // Within some function:
        fgets(..., tainted_fd);
      
      When the engine inlines the definition and finds a function call within
      that, the `PostCall` event for the call will get triggered sooner than the
      `PostCall` for the original function.
      This mismatch violates the assumption of the `GenericTaintChecker` which
      wants to propagate taint information from the `PreCall` event to the
      `PostCall` event, where it can actually bind taint to the return value
      **of the same call**.
      
      Let's get back to the example and go through step-by-step.
      The `GenericTaintChecker` will see the `PreCall<fgets(..., tainted_fd)>`
      event, so it would 'remember' that it needs to taint the return value
      and the buffer, from the `PostCall` handler, where it has access to the
      return value symbol.
      However, the engine will inline fgets and the `nested_call()` gets
      evaluated subsequently, which produces an unimportant
      `PreCall<nested_call()>`, then a `PostCall<nested_call()>` event, which is
      observed by the `GenericTaintChecker`, which will unconditionally mark
      tainted the 'remembered' arg indexes, trying to access a non-existing
      argument, resulting in a crash.
      If it doesn't crash, it will behave completely unintuitively, by marking
      completely unrelated memory regions tainted, which is even worse.
      
      The resulting assertion is something like this:
        Expr.h: const Expr *CallExpr::getArg(unsigned int) const: Assertion
                `Arg < getNumArgs() && "Arg access out of range!"' failed.
      
      The gist of the backtrace:
        CallExpr::getArg(unsigned int) const
        SimpleFunctionCall::getArgExpr(unsigned int)
        CallEvent::getArgSVal(unsigned int) const
        GenericTaintChecker::checkPostCall(const CallEvent &, CheckerContext&) const
      
      Prior to D116025, there was a check for the argument count before it
      applied taint, however, it still suffered from the same underlying
      issue/bug regarding propagation.
      
      This path does not intend to fix the bug, rather start a discussion on
      how to fix this.
      
      ---
      
      Let me elaborate on how I see this problem.
      
      This pre-call, post-call juggling is just a workaround.
      The engine should by itself propagate taint where necessary right where
      it invalidates regions.
      For the tracked values, which potentially escape, we need to erase the
      information we know about them; and this is exactly what is done by
      invalidation.
      However, in the case of taint, we basically want to approximate from the
      opposite side of the spectrum.
      We want to preserve taint in most cases, rather than cleansing them.
      
      Now, we basically sanitize all escaping tainted regions implicitly,
      since invalidation binds a fresh conjured symbol for the given region,
      and that has not been associated with taint.
      
      IMO this is a bad default behavior, we should be more aggressive about
      preserving taint if not further spreading taint to the reachable
      regions.
      
      We have a couple of options for dealing with it (let's call it //tainting
      policy//):
        1) Taint only the parameters which were tainted prior to the call.
        2) Taint the return value of the call, since it likely depends on the
           tainted input - if any arguments were tainted.
        3) Taint all escaped regions - (maybe transitively using the cluster
           algorithm) - if any arguments were tainted.
        4) Not taint anything - this is what we do right now :D
      
      The `ExprEngine` should not deal with taint on its own. It should be done
      by a checker, such as the `GenericTaintChecker`.
      However, the `Pre`-`PostCall` checker callbacks are not designed for this.
      `RegionChanges` would be a much better fit for modeling taint propagation.
      What we would need in the `RegionChanges` callback is the `State` prior
      invalidation, the `State` after the invalidation, and a `CheckerContext` in
      which the checker can create transitions, where it would place `NoteTags`
      for the modeled taint propagations and report errors if a taint sink
      rule gets violated.
      In this callback, we could query from the prior State, if the given
      value was tainted; then act and taint if necessary according to the
      checker's tainting policy.
      
      By using RegionChanges for this, we would 'fix' the mentioned
      propagation bug 'by-design'.
      
      Reviewed By: Szelethus
      
      Differential Revision: https://reviews.llvm.org/D118987
      744745ae
    • Guillaume Chatelet's avatar
      ae8b6386
    • Arthur O'Dwyer's avatar
    • Arthur O'Dwyer's avatar
      [libc++] Remove U+00AD SOFT HYPHEN from comments in tests. NFC. · fc3923fa
      Arthur O'Dwyer authored
          git grep  $(printf '\xc2\xad') ../libcxx
      fc3923fa
    • phyBrackets's avatar
      [analyzer][NFCi] Use the correct BugType in CStringChecker. · 6745b6a0
      phyBrackets authored
      There is different bug types for different types of bugs  but the **emitAdditionOverflowbug** seems to use bugtype **BT_NotCSting** but actually it have to use **BT_AdditionOverflow** .
      
      Reviewed By: steakhal
      
      Differential Revision: https://reviews.llvm.org/D119462
      6745b6a0
    • Aaron Ballman's avatar
      Fix the Sphinx build · f0370827
      Aaron Ballman authored
      Add a heading to appease the Sphinx bot, and add some basic
      documentation for [[_Noreturn]].
      f0370827
    • Aaron Ballman's avatar
      Implement WG14 N2764 the [[noreturn]] attribute · 5029dce4
      Aaron Ballman authored
      This adds support for http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2764.pdf,
      which was adopted at the Feb 2022 WG14 meeting. That paper adds
      [[noreturn]] and [[_Noreturn]] to the list of supported attributes in
      C2x. These attributes have the same semantics as the [[noreturn]]
      attribute in C++.
      
      The [[_Noreturn]] attribute was added as a deprecated feature so that
      translation units which include <stdnoreturn.h> do not get an error on
      use of [[noreturn]] because the macro expands to _Noreturn. Users can
      use -Wno-deprecated-attributes to silence the diagnostic.
      
      Use of <stdnotreturn.h> or the noreturn macro were both deprecated.
      Users can define the _CLANG_DISABLE_CRT_DEPRECATION_WARNINGS macro to
      suppress the deprecation diagnostics coming from the header file.
      5029dce4
    • Momchil Velikov's avatar
      Extend the `uwtable` attribute with unwind table kind · 6398903a
      Momchil Velikov authored
      We have the `clang -cc1` command-line option `-funwind-tables=1|2` and
      the codegen option `VALUE_CODEGENOPT(UnwindTables, 2, 0) ///< Unwind
      tables (1) or asynchronous unwind tables (2)`. However, this is
      encoded in LLVM IR by the presence or the absence of the `uwtable`
      attribute, i.e.  we lose the information whether to generate want just
      some unwind tables or asynchronous unwind tables.
      
      Asynchronous unwind tables take more space in the runtime image, I'd
      estimate something like 80-90% more, as the difference is adding
      roughly the same number of CFI directives as for prologues, only a bit
      simpler (e.g. `.cfi_offset reg, off` vs. `.cfi_restore reg`). Or even
      more, if you consider tail duplication of epilogue blocks.
      Asynchronous unwind tables could also restrict code generation to
      having only a finite number of frame pointer adjustments (an example
      of *not* having a finite number of `SP` adjustments is on AArch64 when
      untagging the stack (MTE) in some...
      6398903a
    • Florian Hahn's avatar
      [DSE] Add additional tests with unreachable exits. · 48f18843
      Florian Hahn authored
      Adds tests for #53800.
      48f18843
    • Simon Pilgrim's avatar
      55b525e9
    • Nikita Popov's avatar
      1aeb4c6b
    • Serguei Katkov's avatar
      506eb6cb
    • Nikita Popov's avatar
      [CGBuilder] Remove CreateBitCast() method · f208644e
      Nikita Popov authored
      Use CreateElementBitCast() instead, or don't work on Address
      where not necessary.
      f208644e
    • Aaron Ballman's avatar
      Check for the overloadable attribute in all the appropriate syntactic locations · 76032b0e
      Aaron Ballman authored
      When forming the function type from a declarator, we look for an
      overloadable attribute before issuing a diagnostic in C about a
      function signature containing only .... When the attribute is present,
      we allow such a declaration for compatibility with the overloading
      rules in C++. However, we were not looking for the attribute in all of
      the places it is legal to write it on a declarator and so we only
      accepted the signature in some forms and incorrectly rejected the
      signature in others.
      
      We now check for the attribute preceding the declarator instead of only
      being applied to the declarator directly.
      76032b0e
    • David Spickett's avatar
      [compiler-rt][xray] Disable fdr-reinit test on Arm · 62c37fa2
      David Spickett authored
      This test is still seemingly randomly segfaulting on Arm:
      https://lab.llvm.org/buildbot/#/builders/178/builds/1547
      
      Though it seems to fail earlier in the test than on AArch64.
      Investigation continues.
      62c37fa2
    • gysit's avatar
      [mlir][linalg] Add attributes to region builder (NFC). · 348bfc8e
      gysit authored
      Adapt the region builder signature to hand in the attributes of the created ops. The revision is a preparation step the support named ops that need access to the operation attributes during op creation.
      
      Depends On D119692
      
      Reviewed By: nicolasvasilache
      
      Differential Revision: https://reviews.llvm.org/D119693
      348bfc8e
    • Nikita Popov's avatar
      [DeadArgElim] Check that function type is the same · 41c5a762
      Nikita Popov authored
      If the function types differ, the call arguments don't necessarily
      correspon to the function arguments. It's likely not worthwhile to
      handle this more precisely, but at least we shouldn't crash.
      41c5a762
    • Marek Kurdej's avatar
      [clang-format] Reformat. NFC. · c72fdad7
      Marek Kurdej authored
      c72fdad7
    • gysit's avatar
      [mlir][OpDSL] Restructure comprehension.py (NFC). · 41210908
      gysit authored
      Group and reorder the classed defined by comprehension.py and add type annotations.
      
      Depends On D119126
      
      Reviewed By: nicolasvasilache
      
      Differential Revision: https://reviews.llvm.org/D119692
      41210908
    • gysit's avatar
      [mlir][OpDSL] Add default value to index attributes. · d50571ab
      gysit authored
      Index attributes had no default value, which means the attribute values had to be set on the operation. This revision adds a default parameter to `IndexAttrDef`. After the change, every index attribute has to define a default value. For example, we may define the following strides attribute:
      ```
      
      ```
      When using the operation the default stride is used if the strides attribute is not set. The mechanism is implemented using `DefaultValuedAttr`.
      
      Additionally, the revision uses the naming index attribute instead of attribute more consistently, which is a preparation for follow up revisions that will introduce function attributes.
      
      Depends On D119125
      
      Reviewed By: stellaraccident
      
      Differential Revision: https://reviews.llvm.org/D119126
      d50571ab
    • Nathan Sidwell's avatar
      [demangler][NFC] Tweak legacy uuidof handling · 880e8758
      Nathan Sidwell authored
      We have to special-case 'u 8__uuidof [tz]' demangling for legacy
      support.  That handling is a little duplicative.
      
      * It seems better to just push the single expected node.
      
      * We can also use 'consumeIf' rather than open-coding the peeking and increment.
      
      * We don't need the numLeft < 2 check, as if there are few than that
        other paths will end up with detecting the error.
      
      FWIW This simplifies a future change adding operator precedence.
      
      Reviewed By: ChuanqiXu
      
      Differential Revision: https://reviews.llvm.org/D119543
      880e8758
    • Nathan Sidwell's avatar
      [demangler] Fix buffer growth · 995c4f30
      Nathan Sidwell authored
      The output buffer growth algorithm had a few issues:
      
      a) An off-by-one error in the initial size check, which uses
      '>='. This error was safe, but could cause us to reallocate when there
      was no need.
      
      b) An inconsistency between the initial size check (>=) and the
      post-doubling check (>).  The latter was somewhat obscured by the
      swapped operands.
      
      c) There would be many reallocs with an initially-small buffer.  Add a
      little initialization hysteresis.
      
      Reviewed By: ChuanqiXu
      
      Differential Revision: https://reviews.llvm.org/D119177
      995c4f30