Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

ATS exposes -O2 bug #22359

Open Quuxplusone opened 9 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR22360
Status NEW
Importance P normal
Reported by Greg Fitzgerald (garious@gmail.com)
Reported on 2015-01-27 16:27:32 -0800
Last modified on 2015-01-28 00:49:15 -0800
Version 3.5
Hardware PC All
CC garious@gmail.com, llvm-bugs@lists.llvm.org, rnk@google.com, t.p.northover@gmail.com
Fixed by commit(s)
Attachments pats_constraint3_solve_dats.c (267209 bytes, application/octet-stream)
Blocks
Blocked by
See also
The ATS community is having trouble with clang 3.5.  Moving from
-O1 to -O2 triggers a crash on startup in the clang-generated
executable.  Hongwei Xi rebuilt with -fsanitize=undefined and
-fsanitize=address and has verified the code has no dependencies on undefined
behavior.
Their workaround is to add a dummy level of indirection to prevent the
[unknown] broken optimization from firing.

https://github.com/githwxi/ATS-Postiats/commit/5e17db4badf58b404598e8b3ae4a666b8b0e889c

Looks like a bug on the LLVM side.  He says it worked with clang 3.2,
but breaks on 3.4 and 3.5 (didn't try 3.3).

Steps to reproduce, build ATS2 with clang 3.5:

    http://www.ats-lang.org/Downloads.html

Ensure your source code does not include the previously-mentioned workaround:

    5e17db4badf58b404598e8b3ae4a666b8b0e889c

Run 'patopt' on the command-line.  If the the bug is present, you should see a
crash on startup.
Quuxplusone commented 9 years ago

Attached pats_constraint3_solve_dats.c (267209 bytes, application/octet-stream): Generated C code

Quuxplusone commented 9 years ago

Typo in the description: 'patopt' -> 'patsopt'

Quuxplusone commented 9 years ago

Can you preprocess the source? I am very curious about the definition of ATS_TRYWITH_TRY. This smells like a setjmp/longjmp/returns_twice bug.

Quuxplusone commented 9 years ago
Very well could be.  Which should it be?  (gotta love that WARNING, eh?)

#define atspre_setjmp(env, mask) setjmp(env)
#define atspre_longjmp(env, ret) longjmp(env, ret)

/*
** WARNING: DO NOT USE THE FOLLOWING MACROS:
*/

#define ATS_TRYWITH_TRY(tmp_exn) \
do { \
ATS_ENTER_EXCEPTION_FRAME() ; \
tmp_exn = (ats_exn_ptr_type)((intptr_t)atspre_setjmp(ATS_CURRENT_FRAME->env,
0)) ; \
if ((intptr_t)tmp_exn == 0) { /* ... */

#define ATS_TRYWITH_WITH(tmp_exn) \
ATS_LEAVE_EXCEPTION_FRAME() ; \
} else { \
tmp_exn = ATS_CURRENT_FRAME->exn ; \
ATS_LEAVE_EXCEPTION_FRAME() ; /* exception handling */

#define ATS_TRYWITH_END() \
} \
} while(0)

/* end of WARNING */
Quuxplusone commented 9 years ago
So this is almost undoubtedly a setjmp issue. Relevant C standard quote
[7.13.2.1 p5]:

"""
All accessible objects have values, and all other components of the abstract
machine249)
have state, as of the time the longjmp function was called, except that the
values of
objects of automatic storage duration that are local to the function containing
the
invocation of the corresponding setjmp macro that do not have volatile-
qualified type
and have been changed between the setjmp invocation and longjmp call are
indeterminate.
"""

As the variable tmp502 is not declared volatile, its value is indeterminate. I
think this is working as intended with nothing to do in LLVM.
Quuxplusone commented 9 years ago
Can you help us understand this better?  The following code is a minor
variation of the Wikipedia's article on setjmp and does not work consistently
between -O1 and -O2.

#include <stdio.h>
#include <stdlib.h>
#include <setjmp.h>

static jmp_buf buf;

void
second(int *p) {
  *p = 1000000;
  printf("second: *p = %i\n", *p);
  longjmp(buf,1);             // jumps back to where setjmp was called - making setjmp now return 1
}

void
first(int *p) {
  second(p);
  printf("first\n");          // does not print
}

int main() {
  /*
  int x = 0;
  */
  int *p;
  p = malloc(sizeof(int));
/*
  printf("main: p = %p\n", p);
*/
  if (p == 0)
  {
    fprintf (stderr, "malloc: failed!\n"); exit(1);
  }

  *p = 0;

  if (!setjmp(buf) ) {
    first(p);                // when executed, setjmp returns 0
  } else {                    // when longjmp jumps back, setjmp returns 1
    printf("main: *p = %i\n", *p);       // prints
  }
  return 0;
}
Quuxplusone commented 9 years ago

Nevermind, I get it.