1. Feb 14, 2022
    • 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 cases the compiler can modify `SP`
      in a loop).
      Having the CFI precise up to an instruction generally also means one
      cannot bundle together CFI instructions once the prologue is done,
      they need to be interspersed with ordinary instructions, which means
      extra `DW_CFA_advance_loc` commands, further increasing the unwind
      tables size.
      
      That is to say, async unwind tables impose a non-negligible overhead,
      yet for the most common use cases (like C++ exceptions), they are not
      even needed.
      
      This patch extends the `uwtable` attribute with an optional
      value:
            -  `uwtable` (default to `async`)
            -  `uwtable(sync)`, synchronous unwind tables
            -  `uwtable(async)`, asynchronous (instruction precise) unwind tables
      
      Reviewed By: MaskRay
      
      Differential Revision: https://reviews.llvm.org/D114543
      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
    • Nikita Popov's avatar
      [Docs] Update OpaquePointers transition state (NFC) · 5a43a278
      Nikita Popov authored
      We're at a point where working optimized binaries can be produced
      in opaque pointer mode.
      5a43a278
    • David Green's avatar
      [ARM] MVE hadd and rhadd · ea6ebbcf
      David Green authored
      This uses the nodes from D106237 to add MVE HADD and RHADD lowering.
      
      Differential Revision: https://reviews.llvm.org/D106238
      ea6ebbcf
    • Anton Afanasyev's avatar
      [SLP] Simplify indices processing for insertelements · 954ea0f0
      Anton Afanasyev authored
      Get rid of non-constant and undef indices of insertelements
      at `buildTree()` stage. Fix bugs.
      
      Differential Revision: https://reviews.llvm.org/D119623
      954ea0f0
    • LLVM GN Syncbot's avatar
      [gn build] Port 55bd22f8 · 31d99229
      LLVM GN Syncbot authored
      31d99229
    • Konstantin Varlamov's avatar
    • gysit's avatar
      [mlir][OpDSL] Consistently use the term op_def (NFC). · 01e04867
      gysit authored
      ... and remove unused type aliases.
      
      Depends On D119003
      
      Reviewed By: nicolasvasilache
      
      Differential Revision: https://reviews.llvm.org/D119125
      01e04867
    • David Green's avatar
      [DAGCombine] Basic combines for AVG nodes. · 03380c70
      David Green authored
      This adds very basic combines for AVG nodes, mostly for constant folding
      and handling degenerate (zero) cases. The code performs mostly the same
      transforms as visitMULHS, adjusted for AVG nodes.
      
      Constant folding extends to a higher bitwidth and drops the lowest bit.
      For undef nodes, `avg undef, x` is transformed to x.  There is also a
      transform for `avgfloor x, 0` transforming to `shr x, 1`.
      
      Differential Revision: https://reviews.llvm.org/D119559
      03380c70
    • Tim Northover's avatar
      Reapply: StackProtector: ignore debug insts when splitting blocks. · a87d3ba6
      Tim Northover authored
      When deciding where to split a block to insert stack guard checks, we should
      move past any debug instructions we see that might (e.g.) be separating a tail
      call from its frame wrangling.
      
      This time, also don't run off the front of a basic block.
      a87d3ba6
    • Peter Waller's avatar
      [gn build] Add host_cpu=arm64 & current_os=linux => aarch64-unknown-linux-gnu · 7f41643e
      Peter Waller authored
      I've been using this triple in development for a while without issues,
      it's passing check-llvm and check-clang.
      
      (The above is the commit message, but the build is currently broken since
      D114639, I intend to submit this once it's passing again and it's accepted in
      review)
      
      Differential Revision: https://reviews.llvm.org/D119331
      7f41643e
    • Nikita Popov's avatar
      [InstCombine] Check GEP source type in select of gep fold · 7c83f8c4
      Nikita Popov authored
      This is no longer implicitly checked through the pointer type
      with opaque pointers.
      7c83f8c4
    • Evgeny Shulgin's avatar
      [clang-tidy] Ignore variable template partial specializations in `misc-definitions-in-headers` · fc84ebff
      Evgeny Shulgin authored
      Variable template partial specializations are inline and can't lead
      to ODR-violations. The checker now ignores them.
      
      Fixes https://github.com/llvm/llvm-project/issues/53519
      
      Reviewed By: hokein
      
      Differential Revision: https://reviews.llvm.org/D119098
      fc84ebff
    • Jean Perier's avatar
      [flang] Fail at link time if derived type descriptors were not generated · 7dd7ccd2
      Jean Perier authored
      Currently, code generation was creating weak symbols for derived type
      descriptor global it could not find in the current compilation unit.
      The rational is that:
       - the derived type descriptors of external module derived types are
         generated in the compilation unit that compiled the module so that
         the type descriptor address is uniquely associated with the type.
       - some types do not have derived type descriptors: the builtin derived
         types used to create derived type descriptors. The runtime knows
         about them and does not need them to accomplish the feat of
         describing themselves. Hence, all unresolved derived type descriptors
         in codegen cannot be assumed to be resolved at link time.
      
      However, this caused immense debugging pain when, for some reasons, derived
      type descriptor that should be generated were not. This caused random
      runtime failures instead of a much cleaner link time failure.
      
      Improve this situation by allowing codegen to detect the builtin derived
      types that have no derived type descriptors and requiring the other
      unresolved derived type descriptor to be resolved at link time.
      
      Also make derived type descriptor constant data since this was a TODO
      and makes the situation even cleaner. This requiring telling lowering
      which compiler created symbols can be placed in read only memory. I
      considered using PARAMETER, but I have mixed feeling using it since that
      would cause the initializer expressions of derived type descriptor to
      be invalid from a Fortran point of view since pointer targets cannot be
      parameters. I do not want to start misusing Fortran attributes, even if
      I think it is quite unlikely semantics would currently complain. I also
      do not want to rely on the fact that all object symbols with the
      CompilerCreated flags are currently constant data. This could easily
      change in the future and cause runtime bugs if lowering rely on this
      while the assumption is not loud and clear in semantics.
      Instead, add a ReadOnly symbol flag to tell lowering that a compiler
      generated symbol can be placed in read only memory.
      
      Differential Revision: https://reviews.llvm.org/D119555
      7dd7ccd2
    • David Green's avatar
      80af78cd
    • Nikita Popov's avatar
      [BitcodeReader] Rename method for element type by ID (NFC) · 4d477ba5
      Nikita Popov authored
      Make it clearer that this method is specifically for pointer
      element types, and not other element types. This distinction will
      be relevant in the future.
      
      The somewhat unusual spelling is to make sure this does not show
      up when grepping for getPointerElementType.
      4d477ba5
    • Nikita Popov's avatar
      [InstCombine] Remove manual debug loc transfer · efece08a
      Nikita Popov authored
      While this might be marginally more precise, we generally don't
      bother with this in InstCombine, and let the IRBuilder assign the
      debug location. I don't see why this one fold, out of the thousands
      done in InstCombine, should be treated specially.
      efece08a
    • Jay Foad's avatar
      [AMDGPU] Fix line endings. NFC. · 9dc43dfa
      Jay Foad authored
      9dc43dfa