CTSRD-CHERI / clang

DO NOT USE. Use llvm-project instead
Other
9 stars 8 forks source link

Casting lambda to std::function crashes back end #148

Closed davidchisnall closed 7 years ago

davidchisnall commented 7 years ago

(Filing here, because I'm not sure whether this is a back end, clang, or libc++ bug, or some combination of the three).

This test case causes the compiler to crash:

#include <functional>

void set(std::function<void()>);

int main()
{
 auto run_gc = [&]()
  {
  };
 set(run_gc);
}

Unfortunately, this is not a very reduced test case, as <functional> is quite large. There are some suspicious things in the generated IR. A lot of { i64, i64, i64, i64 } types are cast to and from pointers.

The generation of lambdas in clang looks correct, so this looks as if it is either a bug in libc++'s implementation of std::function or in clang's handling of the template overloads there.

arichardson commented 7 years ago

Creduces gives me this test case:

// RUN: %clang_cc1 -triple cheri-unknown-freebsd -emit-obj -mrelax-all -mrelocation-model pic -pic-level 2 -mthread-model posix -target-cpu cheri -target-abi purecap -std=c++11 -fdeprecated-macro -fcxx-exceptions -fexceptions -fcolor-diagnostics -cheri-linker -x c++ -o - -w %s | FileCheck %s
inline namespace a {
template <class> struct d;
template <class ab> struct d<ab &> { typedef ab b; };
template <class ab> typename d<ab>::b ag(ab &&) {}
template <long, class> class e;
template <unsigned long...> struct f {};
template <class at, at... au> struct g { template <long> using aw = f<au...>; };
template <long ax, long ay>
using az = typename __make_integer_seq<g, long, ax>::template aw<ay>;
template <int ax, long ay = 0> struct h { typedef az<ax, ay> b; };
template <class...> class i;
template <long bc, class... ab> typename e<bc, i<ab...>>::b bd(i<ab...> &);
template <class> struct H;
template <long bc, class... bf> class e<bc, H<bf...>> {
public:
  typedef __type_pack_element<bc, bf...> b;
};
template <long bc, class... ab> class e<bc, i<ab...>> {
public:
  typedef typename e<bc, H<ab...>>::b b;
};
int bh;
template <class bi> class bj { bi c; };
template <class...> struct j;
template <long... bl, class... ab> struct j<f<bl...>, ab...> : bj<ab>... {};
template <class... ab> class i { j<typename h<sizeof...(ab)>::b, ab...> bm; };
template <class... ab> i<ab &&...> bn(ab...);
template <class bo> class k : bo {
public:
  template <class... br, unsigned long... bs>
  k(int, i<br...> l, i<>, f<bs...>, f<>) : bo(bd<bs>(l)...) {}
};
template <class bo> class m : k<bo> {
public:
  template <class... br, class... bv>
  m(int n, i<br...> l, i<> p3)
      : k<bo>(n, ag(l), p3, typename h<sizeof...(br)>::b(),
              typename h<sizeof...(bv)>::b()) {}
};
template <class> class by;
template <class, class> class cb;
template <class ah, class cd, class... ce> class cb<ah, cd(ce...)> {
  m<ah> cf;

public:
  cb(ah n) : cf(bh, bn(n), bn()) {}
};
template <class cd, class... ce> class by<cd(ce...)> {
public:
  template <class ah, class = void> by(ah);
};
template <class cd, class... ce>
template <class ah, class>
by<cd(ce...)>::by(ah n) {
  cb<ah, cd()> ag(n);
}
}
void ci(by<void()>) {
  auto cj = [] {};
  ci(cj);
}
davidchisnall commented 7 years ago

With the reduced test case, the crash is in the function that looks like this in IR:

; Function Attrs: noinline nounwind optnone
define internal inreg { { i64, i64, i64, i64 } } @"_ZN1a2agIRNS_1iIJOZ2ciNS_2byIFvvEEEE3$_0EEEEENS_1dIT_E1bEOSA_"(%"class.a::i" addrspace(200)* dereferenceable(32)) #0 {
entry:
  %retval = alloca %"class.a::i", align 32, addrspace(200)
  %.addr = alloca %"class.a::i" addrspace(200)*, align 32, addrspace(200)
  store %"class.a::i" addrspace(200)* %0, %"class.a::i" addrspace(200)* addrspace(200)* %.addr, align 32
  call void @llvm.trap()
  unreachable

return:                                           ; No predecessors!
  %coerce.dive = getelementptr inbounds %"class.a::i", %"class.a::i" addrspace(200)* %retval, i32 0, i32 0
  %coerce.dive1 = getelementptr inbounds %"struct.a::j", %"struct.a::j" addrspace(200)* %coerce.dive, i32 0, i32 0
  %coerce.dive2 = getelementptr inbounds %"class.a::bj", %"class.a::bj" addrspace(200)* %coerce.dive1, i32 0, i32 0
  %1 = bitcast %class.anon addrspace(200)* addrspace(200)* %coerce.dive2 to { { i64, i64, i64, i64 } } addrspace(200)*
  %2 = load { { i64, i64, i64, i64 } }, { { i64, i64, i64, i64 } } addrspace(200)* %1, align 32
  ret { { i64, i64, i64, i64 } } %2
}

I'm highly suspicious of the return value of 256-bits of i64, though it appears that the error is actually in the first basic block, near the start:

    %C1<def> = COPY %A0_64

I'm not sure why it thinks that the argument is in $a0, but it appears that something in the calling convention has determined that the argument is in an integer register. The argument is a struct containing a struct containing a struct containing a pointer, so perhaps the lowering logic is broken for this kind of deep nesting?

davidchisnall commented 7 years ago

Reduced test case for the back-end crash:

target datalayout = "E-m:e-pf200:256:256-i8:8:32-i16:16:32-i64:64-n32:64-S128-A200"
target triple = "cheri-unknown-freebsd"

; Function Attrs: noinline nounwind optnone
define { { i64, i64, i64, i64 } } @"_ZN1a2agIRNS_1iIJOZ2ciNS_2byIFvvEEEE3$_0EEEEENS_1dIT_E1bEOSA_"() {
entry:
  call void @llvm.trap()
  unreachable
}

; Function Attrs: noreturn nounwind
declare void @llvm.trap() #4

It appears that we crash because the inreg attribute is present on the { { i64, i64, i64, i64 } } return: this is lowered to a return in $c1 (which is, in fact, what we want), but there's nothing in the back end to correctly handle moving four i64s to a capability register. The real bug is that the return type is { { i64, i64, i64, i64 } } and not a capability.

davidchisnall commented 7 years ago

It appears that MipsABIInfo::HandleAggregates is coercing the return type to integers.

davidchisnall commented 7 years ago

Reduced test case:

class v
{
    void *x = nullptr;
};

class w : public v {};

w foo(void *x)
{
    w v1;
    return v1;
}

This works fine if the return type is v, but it appears that it doesn't handle subclasses correctly.

davidchisnall commented 7 years ago

And the root cause is that the RecordDecl iterators don't iterate over superclass fields, so we ignore them and end up padding with integers.

davidchisnall commented 7 years ago

Note: A fix for this is complicated by the insane MIPS ABI, which passes values from the superclass in integer registers, even if they're floating point values.

Fixed in . df977d4b839ff6a8424eddcca5828f08875552a5

davidchisnall commented 7 years ago

@kgudka Please can you take a look at the fix for this and see if we can use it instead of the big hammer of always passing capability-containing structs by reference?