Fixed compile error on latest GCC (14.2.1)

[NickyBoy89]

This PR contains a fix for a compile error that I noticed while compiling Cobra on my Linux machine:

cc -I. -Wall -O2 -pedantic -Werror -Wshadow -std=c99 -DYY_NO_INPUT -Wformat-truncation=1   -c -o cobra_prim.o cobra_prim.c
cobra_prim.c: In function ‘dogrep’:
cobra_prim.c:158:75: error: ‘%s’ directive output may be truncated writing up to 2030 bytes into a region of size between 0 and 2030 [-Werror=format-truncation=]
  158 |                         snprintf(cmd, sizeof(cmd), "grep -q -n -e  \"%s\" %s",
      |                                                                           ^~
cobra_prim.c:158:25: note: ‘snprintf’ output between 19 and 4079 bytes into a destination of size 2048
  158 |                         snprintf(cmd, sizeof(cmd), "grep -q -n -e  \"%s\" %s",
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  159 |                                 s, f->s);
      |                                 ~~~~~~~~
cc1: all warnings being treated as errors
make: *** [<builtin>: cobra_prim.o] Error 1

It appears that in the dogrep function, the compiler cannot adequately reason about the bounds check that we make to make sure that the resulting command that will be run through grep will not overflow the buffer that was created for it.

This PR separates those steps out into separate variables, and includes a minor refactor to re-copy the grep command into the buffer, instead of modifying the 5th and 6th indexes of the string to remove a flag before it is run again. This I thought made the intent of the code more readable, at the cost of an additional snprintf.

System Details

• OS: Arch Linux, Kernel 6.11.6-arch1-1
• gcc --version

  gcc (GCC) 14.2.1 20240910

[nimble-code]

it's interesting that the modified version doesn't trigger the same warning, since there's no more information there.
I believe this same warning came up in an earlier version but went away when the test just before the snprintf was added.
which version of gcc did you use?

[NickyBoy89]

Hmm, it's interesting that the problem seems intermittent, and seems to be very version-dependent. I remember solving a similar type of issue in #65, just in a different place, so something as simple as separating out the variable name fixes it.

I'm using GCC gcc (GCC) 14.2.1 20240910 (also in the original PR text)

Perhaps it would be worth looking at multiple callsites for snprintf at once next time?

[nimble-code]

I'm using Ubuntu 22.04.2 LTS and gcc version 11.4.0 and do not see any warnings.
I'm thinking of just adding a (redundant) length-test on that one argument to snprintf though, to minimize the change, but I wish that I could see the warning go away....

[NickyBoy89]

Apologies for the late reply. Since these errors have come and gone based on compiler versions, the fact that there are no warnings for you seems very likely.

Since the warning option -Wformat-overflow=1 is a GCC-specific extension, its documentation says:

When the exact number of bytes written by a format directive cannot be determined at compile-time it is estimated based on heuristics that depend on the level argument and on optimization.

Also, the warning specifically seems to find issue with the fact that return values from snprintf aren't used to check the number of bytes written, I can see several ways that we could potentially resolve this.

1. Go through every call site of snprintf to separate out the bounds check variable, leading to code such as this:

int cmd_size = strlen(a) + strlen(b);
if (cmd_size > sizeof(buffer)) {
  exit(1);
}
snprintf(buffer, sizeof(buffer), "%s%s", a, b);

2. Go through every call site and use the return value of snprintf so that we check for bytes written, and don't need the extra bounds check

if (snprintf(buffer, sizeof(buffer), "%s%s", a, b)) >= sizeof(buffer)) {
  exit(1)
}

3. Remove the compiler warning from the Makefile

These are just the options that came to mind, but there likely are better options to fix this issue for good

[nimble-code]

3 isn't too attractive because it might eliminate also accurate warnings.
2 may not work, because it's complaining about arguments being truncated -- the size written cannot exceed the limit of sizeof(buffer) since snprintf already bounds it (unlike sprintf).
so that leaves 1, which comes down to guessing what the gcc analyzer can understand -- will it be able to figure out that the checks on the length of a and b suffice to prevent the truncation?
but yes, with warnings mapped to errors it has to be fixed somehow, even though with changes in gcc versions it may not hold in all cases... difficult case -- perhaps only quiet that gcc warning on those platforms that erroneously issue the warning and hope it resolves itself in due time....

[NickyBoy89]

I just want to point out that 2 might be a a better option than it appears, because GCC says that it will display the warning when:

1. A combination of heuristics cannot check the size of the format string, and
2. The return value is not used

Since 2 does use the return value, this would actually remove this category of errors. But completely up to you!

[nimble-code]

ah -- thanks for pointing that out.
47 calls to snprintf in the code -- none of them check the return value....
would n = snprintf(...); assert(n>0); suppress the warning?
silly, but perhaps it appeases the gcc checker

[NickyBoy89]

From what I've read, I think that would work to catch formatting errors (negative return value), but I think I will draft up another PR to add the checks to this, if solution (2) sounds like the best one

[NickyBoy89]

Continued and rewritten in #76

[Beeram12]

@nimble-code Hey I am facing similar issue how can I resolve it.I cloned the repo and followed the instruction of building but then suddently I faced this error

[Beeram12]

@nimble-code
UPDATE : I modified the line 4275 present in cobra_prog.y

from
rv->val = fprintf((FILE *) vn->pm, q->lft->s);
to
rv->val = fprintf((FILE *) vn->pm, "%s", q->lft->s);
then it's working fine

[nimble-code]

once the update to version 5.0 is complete this issue will also disappear

[Beeram12]

once the update to version 5.0 is complete this issue will also disappear

Hii may I know when can we expect the next version any aprox time.I am trying to implement COBRA in one of my project so a bit of enthusiasm.

[nimble-code]

it's there now

…

On Wed, Mar 19, 2025 at 12:31 PM Pranith_1604 ***@***.***> wrote: once the update to version 5.0 is complete this issue will also disappear Hii may I know when can we expect the next version any aprox time.I am trying to implement COBRA in one of my project so a bit of enthusiasm. — Reply to this email directly, view it on GitHub <#75 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AK6L6IIUPEWJWCXA6OV23GD2VHAZTAVCNFSM6AAAAABRH7SL62VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZXHAZDOOJZHA> . You are receiving this because you were mentioned.Message ID: ***@***.***> [image: Beeram12]*Beeram12* left a comment (nimble-code/Cobra#75) <#75 (comment)> once the update to version 5.0 is complete this issue will also disappear Hii may I know when can we expect the next version any aprox time.I am trying to implement COBRA in one of my project so a bit of enthusiasm. — Reply to this email directly, view it on GitHub <#75 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AK6L6IIUPEWJWCXA6OV23GD2VHAZTAVCNFSM6AAAAABRH7SL62VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZXHAZDOOJZHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>