Bug 485276

Summary: std::optional use: "Conditional jump or move depends on uninitialised value(s)" (clang) aarch64
Product: [Developer tools] valgrind Reporter: m <kde.sigfw>
Component: memcheckAssignee: Paul Floyd <pjfloyd>
Status: REPORTED ---    
Severity: normal CC: mark, pjfloyd
Priority: NOR    
Version First Reported In: 3.22.0   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description m 2024-04-09 13:49:47 UTC 
Similar to https://bugs.kde.org/show_bug.cgi?id=472329

Steps to reproduce with clang-18:


# batcat main.cpp dummy.* && clang++-18 -std=c++17  -g -O2   -I.   main.cpp dummy.cpp -o /tmp/exe && valgrind /tmp/exe

       │ File: main.cpp
   1   │ #include <dummy.h>
   2   │ 
   3   │ #include <optional>
   4   │ #include <set>
   5   │ 
   6   │ static std::set<int> empty_set;
   7   │ 
   8   │ std::optional<dummy> static none()
   9   │ {
  10   │     std::optional<dummy> maybe;
  11   │     {
  12   │         for (const auto& a : empty_set) {
  13   │             if (empty_set.size() > 10) {
  14   │                 maybe.emplace(dummy{});
  15   │             }
  16   │         }
  17   │     }
  18   │     return maybe;
  19   │ }
  20   │ 
  21   │ dummy dummy_or_default()
  22   │ {
  23   │     return none().value_or(dummy{});
  24   │ }
  25   │ 
  26   │ int main()
  27   │ {
  28   │     dummy_or_default();
  29   │ }

       │ File: dummy.cpp
   1   │ #include <dummy.h>
   2   │ 
   3   │ dummy::dummy() = default;

       │ File: dummy.h
   1   │ #include <cstdlib>
   2   │ 
   3   │ struct dummy {
   4   │     char* ptr{};
   5   │ 
   6   │     unsigned zero = 0;
   7   │     bool hp() const { return zero > 0; }
   8   │ 
   9   │     dummy();
  10   │ 
  11   │     ~dummy()
  12   │     {
  13   │         if (hp()) std::free(ptr);
  14   │     }
  15   │ };
==73841== Memcheck, a memory error detector
==73841== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==73841== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==73841== Command: /tmp/exe
==73841== 
==73841== Conditional jump or move depends on uninitialised value(s)
==73841==    at 0x108BB8: _M_reset (optional:317)
==73841==    by 0x108BB8: ~_Optional_payload (optional:441)
==73841==    by 0x108BB8: ~_Optional_base (optional:512)
==73841==    by 0x108BB8: dummy_or_default() (???:23)
==73841==    by 0x108C27: main (main.cpp:28)
==73841== 
==73841== 
==73841== HEAP SUMMARY:
==73841==     in use at exit: 0 bytes in 0 blocks
==73841==   total heap usage: 1 allocs, 1 frees, 73,728 bytes allocated
==73841== 
==73841== All heap blocks were freed -- no leaks are possible
==73841== 
==73841== Use --track-origins=yes to see where uninitialised values come from
==73841== For lists of detected and suppressed errors, rerun with: -s
==73841== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Comment 1 m 2024-04-11 07:43:00 UTC 
Forgot to mention that O1 can be used instead of O2 as a workaround, or clang-16 or earlier should also work as a workaround.
Comment 2 Paul Floyd 2024-04-11 17:51:29 UTC 
With debuginfo it might be OK to suppress this.
Comment 3 Mark Wielaard 2024-04-16 14:44:13 UTC 
I am unable to replicate this with either clang version 17.0.6 or 18.1.2, not with -O2 or -O1.
Comment 4 Mark Wielaard 2024-04-16 14:54:23 UTC 
(In reply to Mark Wielaard from comment #3)
> I am unable to replicate this with either clang version 17.0.6 or 18.1.2,
> not with -O2 or -O1.

On x86_64. Missed that the original report was against arm64 (sorry).
Comment 5 Paul Floyd 2024-04-16 18:07:27 UTC 
I can reproduce this on FreBSD 14 arm64 with clang++ 18.0.0

==14165== Conditional jump or move depends on uninitialised value(s)
==14165==    at 0x210A30: ~__optional_destruct_base (optional:247)
==14165==    by 0x210A30: dummy_or_default() (bug485276.cpp:23)
==14165==    by 0x210A9F: main (bug485276.cpp:28)
Comment 6 Paul Floyd 2024-04-16 19:27:55 UTC 
│      244      _LIBCPP_INLINE_VISIBILITY
│      245      _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__optional_destruct_base()
│      246      {
│  >   247          if (__engaged_) 
│      248              __val_.~value_type();
│      249      } 

Not sure what is happening. The error occurs as above, but it's not just __engaged_ that is not initialized, this points to an uninitialized object.

(gdb) mc xb &__engaged_ sizeof(__engaged_)
                  ff
0x1FFFFFF8F0:   0x60

(gdb) mc xb this sizeof(*this)
                  00      ff      ff      ff      ff      ff      ff      ff
0x1FFFFFF8E0:   0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
                  ff      ff      ff      ff      ff      ff      ff      ff
0x1FFFFFF8E8:   0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
                  ff      ff      ff      ff      ff      ff      ff      ff
0x1FFFFFF8F0:   0x60    0xf9    0xff    0xff    0x1f    0x00    0x00    0x00


type = struct std::__1::__optional_destruct_base<dummy, false> [with _Tp = dummy] {
    union {
        char __null_state_;
        value_type __val_;
    };
    bool __engaged_;
  public:
    ~__optional_destruct_base(void);
    __optional_destruct_base(void);
    void reset(void);

    typedef _Tp value_type;
}

'this' points to memory on the stack which is why it's safe to dereference.

I'm not familiar with the code, but as I understand it, this is the key function:

static Bool stmt_is_guardable ( const IRStmt* st )
{
   switch (st->tag) {
      // These are easily guarded.
      case Ist_NoOp:
      case Ist_IMark:
      case Ist_Put:
      case Ist_PutI:
         return True;
      // These are definitely not guardable, or at least it's way too much
      // hassle to do so.
      case Ist_CAS:
      case Ist_LLSC:
      case Ist_MBE:
         return False;
      // These could be guarded, with some effort, if really needed, but
      // currently aren't guardable.
      case Ist_LoadG:
      case Ist_Store:
      case Ist_StoreG:
      case Ist_Exit:
      case Ist_Dirty:
         return False;
      // This is probably guardable, but it depends on the RHS of the
      // assignment.
      case Ist_WrTmp:
         return expr_is_guardable(st->Ist.WrTmp.data);
      default:
         vex_printf("\n"); ppIRStmt(st); vex_printf("\n");
         vpanic("stmt_is_guardable: unhandled stmt");
   }
}