| Message ID | CACysShih6q6rzeUZ58_e-iDzXF-jC8=2u8kvHm-+tnkJSDJEew@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Alexander, just a couple of nits: > gcc/testsuite/ChangeLog: > > * gcc.target/i386/mpx/PR79988.c: New test. We usually don't use capital PR in testcase names. Please use pr79988.c instead, matching the other files there. Also, both ChangeLog entries should include a PR reference like so: PR middle-end/79988 * gcc.target/i386/mpx/pr79988.c: New test. so the commit messages are automatically forwarded to bugzilla. > gcc/ChangeLog: > > * tree-chkp.c (chkp_gimple_call_builtin_p): > Remove gimple_call_builtin_p call to avoid the call > of gimple_builtin_call_types_compatible_p. this will > strip the checks for address spaces, which can be skipped > without loosing the functionality ChangeLog entries only describe *what* changed, not *why*. Thanks. Rainer
Hi, Please put comment to code explaining why you don't use gimple_call_builtin_p to avoid similar issues in the future. Also please follow Rainer's comments. OK with these fixes. Thanks, Ilya 2017-04-20 15:27 GMT+03:00 Rainer Orth <ro@cebitec.uni-bielefeld.de>: > Hi Alexander, > > just a couple of nits: > >> gcc/testsuite/ChangeLog: >> >> * gcc.target/i386/mpx/PR79988.c: New test. > > We usually don't use capital PR in testcase names. Please use pr79988.c > instead, matching the other files there. > > Also, both ChangeLog entries should include a PR reference like so: > > PR middle-end/79988 > * gcc.target/i386/mpx/pr79988.c: New test. > > so the commit messages are automatically forwarded to bugzilla. > >> gcc/ChangeLog: >> >> * tree-chkp.c (chkp_gimple_call_builtin_p): >> Remove gimple_call_builtin_p call to avoid the call >> of gimple_builtin_call_types_compatible_p. this will >> strip the checks for address spaces, which can be skipped >> without loosing the functionality > > ChangeLog entries only describe *what* changed, not *why*. > > Thanks. > Rainer > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
diff --git a/gcc/testsuite/gcc.target/i386/mpx/PR79988.c b/gcc/testsuite/gcc.target/i386/mpx/PR79988.c new file mode 100644 index 0000000..a6e43eb --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mpx/PR79988.c @@ -0,0 +1,6 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */ + +void foo(unsigned char * __seg_gs *pointer_gs) { + pointer_gs[5] = 0; +} diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index b1ff218..1f7184d 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -433,7 +433,9 @@ chkp_gimple_call_builtin_p (gimple *call, enum built_in_function code) { tree fndecl; - if (gimple_call_builtin_p (call, BUILT_IN_MD) + if (is_gimple_call (call) + && (fndecl = gimple_call_fndecl (call)) != NULL + && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD && (fndecl = targetm.builtin_chkp_function (code)) && (DECL_FUNCTION_CODE (gimple_call_fndecl (call)) == DECL_FUNCTION_CODE (fndecl)))
Hi Ilya, The patch below is basically what Richard proposed in Bugzilla. This approach should produce the correct code for any address spaces, because it will just strip the address space part of the pointer in all bnd* instructions; Since we will do that consistently, all checks should be consistent and correct. What do you think? gcc/testsuite/ChangeLog: * gcc.target/i386/mpx/PR79988.c: New test. gcc/ChangeLog: * tree-chkp.c (chkp_gimple_call_builtin_p): Remove gimple_call_builtin_p call to avoid the call of gimple_builtin_call_types_compatible_p. this will strip the checks for address spaces, which can be skipped without loosing the functionality