-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
In <caml/major_gc.h> declare some variables 'extern' #1770
Conversation
The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables.
@damiendoligez please review ASAP, this is your code, and I'd like the fix to go in 4.07. |
And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). As explained in commit 74ce561, it is preferable to declare variables "extern" in .h files and have them declared non-extern only once, in a .c file.
For completeness I used clang's warnings to find other variables that should be |
Reviewers are hard to find and code is obviously correct, so I take responsibility for merging. |
GPR #1770. * In <caml/major_gc.h> declare some variables 'extern' The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above.
Cherry-pciked in 4.07, 1161b77 |
Note that COMMON data is a source of other problems anyway (https://caml.inria.fr/mantis/view.php?id=5693). |
GPR #1770. * In <caml/major_gc.h> declare some variables 'extern' The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above. (cherry picked from commit 1161b77)
GPR ocaml#1770. * In <caml/major_gc.h> declare some variables 'extern' The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above.
GPR ocaml#1770. * In <caml/major_gc.h> declare some variables 'extern' The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above. (cherry picked from commit 1161b77)
GPR ocaml#1770. * In <caml/major_gc.h> declare some variables 'extern' The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above. (cherry picked from commit 1161b77)
GPR ocaml#1770. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above. (cherry picked from commit 1161b77)
GPR ocaml#1770. * In <caml/major_gc.h> declare some variables 'extern' The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit. It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker. However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest. This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above. (cherry picked from commit 1161b77)
GPR ocaml#1770. * In <caml/intext.h> declare `caml_code_fragments_table` as extern And add proper declaration in byterun/fix_code.c and asmrun/startup.c (the places where this table is initialized). Same reasons as above. (cherry picked from commit 1161b77)
The variables in question are caml_major_ring, caml_major_ring_index, and caml_major_work_credit.
It is correct C code to declare those variables in the .h without a storage class, in which case they are treated as "common" variables by the linker.
However, this causes problems with Clang's address sanitizer: it increases the size of the common variable when it is used, but leaves it unchanged when it is not. As a consequence, the linker sees several occurrences of a "common" variable with different sizes. The linker does the right thing and takes the "max" of the sizes, but not without printing an annoying warning, which is then reported as suspicious behavior by ocamltest.
This commit puts "extern" storage class on the incriminated variables in <caml/major_gc.h>, consistently with the other GC variables.