Menu

#868 Print a diagnostic message when _sdcc_external_startup is encountered

None
closed-fixed
None
5
2023-02-04
2023-02-03
No

In the light of [bugs:#3544] it is probably a good idea to print a diagnostic message of either warning or error type when the old spelling of __sdcc_external_startup with only one underscore is encountered.
Despite the better standard compliance of the new identifier, the very specific way it is used can silently break existing code, which should be avoided, i.e. made non-silent at least for a transition period.

Maybe a deliberately incompatible internal function declaration could do that job.
Maybe we need something more specific, though.

Related

Bugs: #3544
Wiki: NGI0-Entrust-SDCC

Discussion

  • Basil Hussain

    Basil Hussain - 2023-02-03

    If the renaming will be a significant issue for all platforms, then I agree that getting SDCC to emit a warning when a startup function under the old name is found would be a good move. But obviously it's hard to estimate the potential impact. Perhaps a survey on the users mailing list would be in order?

    With regard to the specific problem with the PDK platform and Free-PDK project headers, documentation, and examples, I think the areas of concern are two-fold:

    • Users who go to compile existing codebases containing the old function name with the latest SDCC.
    • Users who follow any up-to-date docs or examples that specify the use of the new name, but who are using an older SDCC version.

    In both cases, the user will encounter transparent (i.e. without warning or error) failure or misbehaviour due to omission of system clock initialisation and calibration that is typically done inside the startup function when using the Free-PDK headers.

    In the first case, only a warning by SDCC can realistically help. In the second case, additions to the Free-PDK includes can help there, but SDCC cannot.

    So, we may need a two-pronged attack to mitigate any potential problems for the PDK platform.

     
  • Philipp Klaus Krause

    I guess we could temporarily (for a few SDCC releases) emit a warning when we encounter a function named _sdcc_external_startup.

     
  • Basil Hussain

    Basil Hussain - 2023-02-03

    I have devised a way of adding a compile-time check for the old function name to the Free-PDK headers, so I have submitted a pull request to them: https://github.com/free-pdk/pdk-includes/pull/14

    That should help for the situation where new code is created using the old name, or where existing code is updated to use the latest Free-PDK headers, but is obviously not a complete solution.

     

    Last edit: Basil Hussain 2023-02-04
    • Free-PDK

      Free-PDK - 2023-02-04

      Can you explain the code in your pull request?

      If I read it correctly then

      • you add a check to a single MACRO which is only related partially to the problem
      • you check for SDCC version >= revision with breaking change
      • also you check if the NEW version __sdcc_external_startup is defined
      • and if so then emit a warning

      How should this warn people which do NOT define the new version __sdcc_external_startup?

      Can you really check in assembly for a function definition with .ifdef? And if so, why no extra _ needs to be added like to all other function definitions coming from C ==> ___sdcc_external_startup?

       

      Last edit: Free-PDK 2023-02-04
      • Basil Hussain

        Basil Hussain - 2023-02-04

        you add a check to a single MACRO which is only related partially to the problem

        The PDK_SET_SYSCLOCK() macro is merely a convenient point at which to make the check, seeing as it is meant to be used within the startup function in question. The check is a compile-time check - the function's code doesn't actually have to run.

        also you check if the NEW version __sdcc_external_startup is defined

        You have mis-interpreted the code. It checks for the old function name. SDCC, when compiling C into assembly, prefixes all function names with an underscore. Therefore, in the context of inline assembly, _sdcc_external_startup becomes __sdcc_external_startup (1 underscore to 2), and __sdcc_external_startup becomes ___sdcc_external_startup (2 underscores to 3).

        Can you really check in assembly for a function definition with .ifdef?

        Yes. The .ifdef directive tests for a symbol with a defined value. A C function's entry point is an assembly label, which is usable as just another symbol whose value is equal to its location count (i.e. program address). When there is no function by the given name, no symbol exists, so the .ifdef expression is false.

        Checking for functions this way is of course limited to the specific translation unit the inline assembly ends up in, so it cannot be program-wide.

        And if so, why no extra _ needs to be added like to all other function definitions coming from C ==> ___sdcc_external_startup?

        The extra underscore is added, but you mis-read the code. See above.

        (By the way, if we are going to discuss the GitHub pull request, please let us do it on GitHub, in the pull request's comments. It would be helpful for discussion here to remain limited to the modifications to SDCC.)

         
  • Philipp Klaus Krause

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

    Implemented in [r13850].

     

    Related

    Commit: [r13850]


Log in to post a comment.

MongoDB Logo MongoDB