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

crash in shouldDiscardValueNames #133

Closed zyz9740 closed 8 months ago

zyz9740 commented 2 years ago

crash_shouldDiscardValueNames.zip

You can try this case.

zyz9740 commented 2 years ago

A more consice version:

#include <stdio.h>

int main() {
  strcpy(0, "cd");
  return 0;
}

It's indeed invalid usage of strcpy function but may not cause crash

carlopi commented 2 years ago

Hi, here there are two separate problems:

  1. the implicit assumption that IRBuilder<>::CreateGEP will actually create a GEP (e.g. https://github.com/leaningtech/cheerp-compiler/blob/master/llvm/lib/CheerpUtils/StructMemFuncLowering.cpp#L103), while potentially it might get folded into a Constant, and for the the solution is using IRBuilder (see https://github.com/leaningtech/cheerp-compiler/commit/9e9e5272b0624890a944480e1eb58a0a0afc73a2, but I have to review it before committing).

  2. the implicit assumption in the JavaScript writer that no malformed code will get there, in this case since the call to strcpy requires a non-null argument, I am unsure whether we should be robust to that case or accept a runtime failure (currently the output looks like this: null[0][0]=HEAP16[(Lgeptoindexphi<<1)+1050858>>1]|0;, and will trigger a JavaScript TypeError: Cannot read properties of null (reading 0) when executing that code)

I will check with the team on both. Thanks

Hyxogen commented 8 months ago

Although neither crashes seemed to be happening anymore, the case attached in the zip did cause a hang, which was fixed in: https://github.com/leaningtech/cheerp-compiler/commit/0efaba0e382e4d3f6d69d615357a22eb4b5eeb7d