Re: [HACKERS] [PATCH] Code refactoring related to -fsanitize=use-after-scope
On 02/15/2016 08:20 PM, Tom Lane wrote: > I bet a nickel that this is triggered by the goto leading into those > variables' scope ("goto process_inner_tuple" at line 2038 in HEAD). > That probably bypasses the "unpoison" step. > > However, doesn't this represent a bug in the sanitizer rather than > anything we should change in Postgres? There is no rule in C that > you can't execute such a goto, especially not if there is no > initialization of those variables. > > If you can think of a reasonable refactoring that gets rid of the need > for that goto, I'd be for that, because it's certainly unsightly. > But I don't think it's wrong, and I don't think that the proposed patch > is any improvement from a structured-programming standpoint. > > regards, tom lane Hi Tom. You are exactly right that as the code does not expose an initialization, it should work fine. As you mentioned, unpoisoning is skipped that exposes this false positive. I'll try to think about the case and handle that. Application of my patch does not make sense. Martin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Code refactoring related to -fsanitize=use-after-scope
Andres Freund writes: > On 2016-02-15 14:37:28 +0100, Martin Liška wrote: >> I've been currently working on support of -sanitize=use-after-scope in the >> GCC compiler and >> I decided to use postgresql as my test-case. The sanitation poisons every >> stack variable at the >> very beginning of a function, unpoisons a variable at the beginning of scope >> definition and finally >> poisons the variable again at the end of scope. > Generally sounds like a good check. >> Following patch fixes issues seen by the sanitizer. Hope it's acceptable? >> With the patch applied, ASAN (with the new sanitization) works fine. > But I'm not immediately seing why this is necessary? Is this about > battling a false positive? I bet a nickel that this is triggered by the goto leading into those variables' scope ("goto process_inner_tuple" at line 2038 in HEAD). That probably bypasses the "unpoison" step. However, doesn't this represent a bug in the sanitizer rather than anything we should change in Postgres? There is no rule in C that you can't execute such a goto, especially not if there is no initialization of those variables. If you can think of a reasonable refactoring that gets rid of the need for that goto, I'd be for that, because it's certainly unsightly. But I don't think it's wrong, and I don't think that the proposed patch is any improvement from a structured-programming standpoint. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Code refactoring related to -fsanitize=use-after-scope
Hi, On 2016-02-15 14:37:28 +0100, Martin Liška wrote: > I've been currently working on support of -sanitize=use-after-scope in the > GCC compiler and > I decided to use postgresql as my test-case. The sanitation poisons every > stack variable at the > very beginning of a function, unpoisons a variable at the beginning of scope > definition and finally > poisons the variable again at the end of scope. Generally sounds like a good check. > Following patch fixes issues seen by the sanitizer. Hope it's acceptable? > With the patch applied, ASAN (with the new sanitization) works fine. > diff --git a/src/backend/access/spgist/spgdoinsert.c > b/src/backend/access/spgist/spgdoinsert.c > index f090ca5..ff986c2 100644 > --- a/src/backend/access/spgist/spgdoinsert.c > +++ b/src/backend/access/spgist/spgdoinsert.c > @@ -1871,6 +1871,10 @@ spgdoinsert(Relation index, SpGistState *state, > SPPageDesc current, > parent; > FmgrInfo *procinfo = NULL; > + SpGistInnerTuple innerTuple; > + spgChooseIn in; > + spgChooseOut out; > + > > /* >* Look up FmgrInfo of the user-defined choose function once, to save > @@ -2044,9 +2048,6 @@ spgdoinsert(Relation index, SpGistState *state, >* Apply the opclass choose function to figure out how > to insert >* the given datum into the current inner tuple. >*/ > - SpGistInnerTuple innerTuple; > - spgChooseIn in; > - spgChooseOut out; But I'm not immediately seing why this is necessary? Is this about battling a false positive? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Code refactoring related to -fsanitize=use-after-scope
Hello. I've been currently working on support of -sanitize=use-after-scope in the GCC compiler and I decided to use postgresql as my test-case. The sanitation poisons every stack variable at the very beginning of a function, unpoisons a variable at the beginning of scope definition and finally poisons the variable again at the end of scope. Following patch fixes issues seen by the sanitizer. Hope it's acceptable? With the patch applied, ASAN (with the new sanitization) works fine. Thanks, Martin diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.c index f090ca5..ff986c2 100644 --- a/src/backend/access/spgist/spgdoinsert.c +++ b/src/backend/access/spgist/spgdoinsert.c @@ -1871,6 +1871,10 @@ spgdoinsert(Relation index, SpGistState *state, SPPageDesc current, parent; FmgrInfo *procinfo = NULL; + SpGistInnerTuple innerTuple; + spgChooseIn in; + spgChooseOut out; + /* * Look up FmgrInfo of the user-defined choose function once, to save @@ -2044,9 +2048,6 @@ spgdoinsert(Relation index, SpGistState *state, * Apply the opclass choose function to figure out how to insert * the given datum into the current inner tuple. */ - SpGistInnerTuple innerTuple; - spgChooseIn in; - spgChooseOut out; /* * spgAddNode and spgSplitTuple cases will loop back to here to -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers