Menu ▾ ▴

#4109 No recommended diagnostic for &* when its result is returned through a local pointer

closed-fixed
None
Front-end
5
1 day ago
2 days ago
No

1: Sample code that reproduces the problem.

int *foo(_Optional int *poi)
{
    int *pi = &*poi;
    return pi;
}

2: Exact command used to run SDCC on this sample code.

sdcc --std=c23 -c test.c

3: SDCC version tested.

SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/r4k/r5k/r6k/sm83/tlcs90/ez80/z80n/r800/ds390/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15 TD- 4.6.4 #16956 (Linux)

The same behaviour was observed with SDCC 4.6.0.

4: Actual output and expected behaviour.

No diagnostic output is produced.

Nothing in this function establishes that poi is non-null, so I expected warning 355 at &*poi.

The unary & removes _Optional from the referenced type of the resulting pointer. The initialisation of pi and the return statement therefore do not discard a qualifier.

Replacing the body with return &*poi; produces the expected diagnostic with the same compiler:

test.c:3: warning 355: pointer to _Optional could not be proven to be non-null at dereference

The initial iCode has isSemDeref set on the source operand of the assignment to pi. CSE substitutes the parameter value for pi in the return statement, without transferring that flag. Dead-code elimination then removes the assignment containing the flag before the diagnostic check runs.

Related

Bugs: #4003
Bugs: #4006

Discussion

  • Christopher Bazley

    The diagnostic is lost when CSE replaces the local pointer at its use and dead-code elimination removes the marked assignment before the diagnostic pass. The attached preliminary fix preserves the marked instruction until that pass, then clears the marker and removes dead code before loop optimisation. It adds 22 lines to SDCCopt.c and requires no new iCode operation or backend changes. ('Preliminary' because it does not also fix the bug#4003 case. I think the attached patch is worth considering as an interim solution though, if only because it does fix other cases, and it adds a lot of useful test coverage.)

     

    Last edit: Christopher Bazley 2 days ago
  • Philipp Klaus Krause

    • status: open --> closed-fixed
    • assigned_to: Philipp Klaus Krause
     
  • Philipp Klaus Krause

    Thanks. Fixed in [r16961] via your patch (with an added comment in the test, that we might want to enable it for the host compilers, if those get _Optional support). Nice that there is no code size regression in the regression tests, despite an optimisation being done a bit later now.

     

    Related

    Commit: [r16961]


Log in to post a comment.