ut icon indicating copy to clipboard operation
ut copied to clipboard

SEGV when using Clang [12, 16]

Open pfeatherstone opened this issue 1 year ago • 15 comments

Expected Behavior

I expect the library not to SIGSEGV fault

Actual Behavior

It SIGSEGV faults when compiling with Clang 12, 13, 14, 15, 16. It looks to be ok with 17 and 18.

Steps to Reproduce the Problem

https://godbolt.org/z/avKr3dj6j

Specifications

  • Version: Master
  • Platform: Any X86 platform

pfeatherstone avatar Jul 28 '24 13:07 pfeatherstone

@krzysztof-jusiak Also, are you moving development to https://github.com/qlibs/ut ? Or are they two separate things now? I was having trouble getting ut2 to work with runtime tests.

pfeatherstone avatar Jul 28 '24 13:07 pfeatherstone

@krzysztof-jusiak Have you had a chance to look at this?

pfeatherstone avatar Aug 01 '24 07:08 pfeatherstone

Thanks @pfeatherstone for pointing it out. I've looked but I haven't fixed it yet and got distracted (there is UB in the string_view name assignment to be fixed, not sure how it got introduced and why actions didn't catch it). BTW the issue doesn't happen with -stdlib=libc++. Looking again. BTW regarding qlibs/ut it's a different project (Based on experience with boost-ext/ut) but written from scratch with different running model (tests by default are at executed at compile-time) also it's focused on being lite and fast compilation times.

kris-jusiak avatar Aug 01 '24 08:08 kris-jusiak

Thank you for your response. I gave qlibs/ut a go but I had to do a lot of fighting in order to get things to compile and run tests at runtime. Maybe it's more user-friendly for compile-time tests whereas boost-ext/ut is more friendly towards runtime tests??

pfeatherstone avatar Aug 01 '24 08:08 pfeatherstone

I see the issue with UT, it's uninitialized variable and string_view pointing to already dead string. for qlibs/ut, there are different modes, to run tests at run-time you can simply just put mutable on the lambda or compile with -DUT_RUNTIME_ONLY which will run all tests at run-time.

static_assert(("sum"_test = [] { // compile-time only
  expect(sum(1, 2, 3) == 6_i);
}));

int main() {
  "sum"_test = [] {              // compile time and run-time
    expect(sum(1, 2, 3) == 5_i);
  };

  "sum"_test = [] constexpr {    // compile-time and run-time
    expect(sum(1, 2, 3) == 6_i);
  };

  "sum"_test = [] mutable {      // run-time only
    expect(sum(1, 2, 3) == 6_i);
  };

  "sum"_test = [] consteval {    // compile-time only
    expect(sum(1, 2, 3) == 6_i);
  };
}

kris-jusiak avatar Aug 01 '24 08:08 kris-jusiak

Awesome I'll give qlibs an other go later. Otherwise, it would be great to get that bug fixed

pfeatherstone avatar Aug 01 '24 15:08 pfeatherstone

Hmm, the more I look at it the more I'm convincing myself it's actually a clang bug as the generated assembly seems wrong for static variables causing the memory being overwritten, but need to confirm that with smaller repro case.

kris-jusiak avatar Aug 02 '24 10:08 kris-jusiak

Do you think qlibs/ut2 would suffer from the same problem?

pfeatherstone avatar Aug 02 '24 10:08 pfeatherstone

no, it's related to how clang handles static storage, either way I'm going to fix/workaround it. qlibs/ut is much simpler in that regards so doesn't suffer from it.

kris-jusiak avatar Aug 02 '24 10:08 kris-jusiak

I have found other issues with ut2 when using clang https://github.com/qlibs/ut/issues/3

pfeatherstone avatar Aug 02 '24 11:08 pfeatherstone

I might remove Clang from my CI. Or i'm sorry to say, move back to doctest. As much as I love this library, it requires a very modern compiler.

pfeatherstone avatar Aug 02 '24 11:08 pfeatherstone

May consider moving to 1.1.9 version which is stable and works with previous clang versions (no junit reporter though)

  • https://godbolt.org/z/P6nr1TWxj

kris-jusiak avatar Aug 02 '24 12:08 kris-jusiak

I think this might be a symptom of a larger issue that seems ODR related.

In a library I'm working on, using UT across separate translation units causes segfaults with clang (16 thru 18, maybe others).

However, when I combine all tests into a single TU, where the boost::ut::cfg is instantiated manually, before any tests are defined, and the runner is invoked manually in main, the segfaults are eliminated.

It has to be the full combination of single TU, manual instantiation, and manual run to eliminate the segfaults though. Neglect any one of those, and * boom *.

While tracking down that combination to success, I also caused MSVC to experience the same segfault (but unfortunately I didn't note down the combination needed to produce that one), so I don't think this is clang-specific, it's just that clang is the least tolerant / most likely to blow up.

braxtons12 avatar Sep 23 '24 01:09 braxtons12

While tracking down that combination to success, I also caused MSVC to experience the same segfault (but unfortunately I didn't note down the combination needed to produce that one), so I don't think this is clang-specific, it's just that clang is the least tolerant / most likely to blow up.

Looking back through the CI and my git history on that branch, it looks like the combination for this was manual instantiation of boost::ut::cfg in the TU main is in, manual run in main, but tests defined in multiple TUs

braxtons12 avatar Sep 23 '24 01:09 braxtons12

I think this might be a symptom of a larger issue that seems ODR related.

Pretty sure this is the case. The library stopped working reliably for me at some point in the last year or so, I'm afraid I don't have a clear record of what version, or change at my end triggered it. I began an investigation a while back but didn't have the time to get to the bottom of it. However my preliminary conclusion was that the way the library is combining templated variables (e.g. cfg) with static initialization is not conformant C++. It's relying on assumptions about initialization order sequencing that as far as I understand do not hold (or anyway are not guaranteed by the standard) for variable templates.

Debugging showed the suites container being written to when test initializers are run, before it's been initialized. Then afterwards that suites variable has its own initializer run, stomping on the earlier writes. This manifests as a mixture of segfaults and sometimes just running but skipping tests.

kamrann avatar Dec 28 '24 05:12 kamrann