ld/dwarf: raise includestack to 256, clamp instead of abort#144
Conversation
APE headers have deep transitive include chains that exceed the original Go-era assumption. Raise the limit to 256 (covers real-world APE programs) and change checknesting() to clamp the stack rather than calling errorexit() — DWARF line info may be slightly imprecise for extreme nesting but the link succeeds. https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request increases the DWARF include stack size from 64 to 256 to handle deep transitive include chains in APE headers and modifies the nesting check to truncate line info rather than terminating. However, the reviewer identified that the current clamping logic causes diagnostic spam and risks a stack underflow when the nesting level decreases, as the stack pointer loses synchronization with the actual depth. A suggestion was provided to use a static flag for the warning and to preserve the stack trace to aid in debugging.
| if(includetop == nelem(includestack)) | ||
| diag("include nesting too deep, truncating DWARF line info"); | ||
| includetop = nelem(includestack) - 1; |
There was a problem hiding this comment.
The clamping logic introduced here has two significant issues:
-
Diagnostic Spam: Since
includetopis reset tonelem(includestack) - 1, the next push will increment it back tonelem, triggering thediagmessage again. This will repeat for every include level and line change beyond the limit, flooding the output. -
Stack Underflow / Desynchronization: Clamping the stack pointer
includetopcauses it to lose sync with the actual nesting depth. When the nesting level decreases (pops),includetopwill be decremented from the clamped value. Once the nesting returns to zero,includetopwill become negative, likely triggering a fatal error (stack underflow) later in the execution. This defeats the purpose of making the limit non-fatal.
To fix the spam, use a static flag. It is also recommended to keep the stack trace (the loop over histfile) inside the if(!warned) block to help identify the cause of the deep nesting. To properly handle the nesting without aborting, you should ideally allow includetop to track the real depth but guard the array accesses to use a clamped index.
static int warned;
if(!warned){
int i;
diag("include nesting too deep, truncating DWARF line info");
for(i = 0; i < nelem(includestack); i++)
diag("%s", histfile[includestack[i].file]);
warned = 1;
}
includetop = nelem(includestack) - 1;2 participants
APE headers have deep transitive include chains that exceed the original Go-era assumption. Raise the limit to 256 (covers real-world APE programs) and change checknesting() to clamp the stack rather than calling errorexit() — DWARF line info may be slightly imprecise for extreme nesting but the link succeeds.
https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs