leaningtech / cheerp-meta

Cheerp - a C/C++ compiler for Web applications - compiles to WebAssembly and JavaScript
https://labs.leaningtech.com/cheerp
Other
1.03k stars 51 forks source link

Default member initializers works incorrectly with `std::vector` #97

Closed yeputons closed 4 years ago

yeputons commented 4 years ago

Consider the following example:

#include <iostream>
#include <vector>

struct Foo {
    std::vector<int> field{10000};
};

void webMain() {
    Foo f;
    std::cout << "size=" << f.field.size() << "\n";
    f.field.at(1000) = 239;
    std::cout << f.field.at(1000) << "\n";
    std::cout << f.field.at(1000) << "\n";
}

I expect it to print:

size=10000
239
239

However, when compiled with -target=cheerp it prints

size=1
0
0

and when compiled with -target=cheerp-wasm it prints

size=1
239
239

Looks like the default member initializer is called for f.field with wrong size. Moreover at() does not raise an error when accessing an invalid element.

Cheerp version:

Cheerp 2.5rc1 clang version 6.0.0 (https://github.com/leaningtech/cheerp-clang.git 82d82445b8e7a6790ac7c1ef3aa1ebbb3f749f1c) (https://github.com/leaningtech/cheerp-llvm.git a3bfe0080e64054be6bcc5d484770f05c548b934) (based on LLVM 6.0.0git-a3bfe008)
Target: i686-pc-windows-gnu
Thread model: posix
InstalledDir: c:\cheerp\bin
carlopi commented 4 years ago

Hi @yeputons, There are a few things that are going on here:

-initializer list ({}) semantics for vectors dictate that you create a std::vector with a single element 10000 here. What you probably want to do is:

#include <iostream>
#include <vector>

struct Foo {
    std::vector<int> field{std::vector<int>(10000)};
};

int main() {
    Foo f;
    std::cout << "size=" << f.field.size() << "\n";
    f.field.at(1000) = 239;
    std::cout << f.field.at(1000) << "\n";
    std::cout << f.field.at(1000) << "\n";
}

(main and webMain are basically interchangeable for cheerp, using main you can compile the exact same code with regular clang++ / g++ and with Cheerp and check whether the behavior is the same)

-in your initial test, the one with the vector initialized to a single element, I should investigate what's going on with the call to at(), the thing most similar to a solution here is passing the flag -cheerp-bounds-check + -target cheerp, and in this case it would raise a JavaScript exception OutOfBound. What's most likely happening behind the scene is that your file get's linked with the release version of the libc++, and there function member at() get's basically degraded to operator[]() (so losing the checks).

yeputons commented 4 years ago

Oh, sure, my bad. Thank you for pointing that out.

-cheerp-bounds-check + -target cheerp work well together.

.at() should not degrade even in release mode, it's required to throw std::out_of_range on bad index. However, since Cheerp does not support exceptions, it's not really clear what to do. Looks like a separate issue.