OCamlPro / gnucobol

A clone of the sourceforge GnuCOBOL compiler from COBOL to C.
https://get-superbol.com
GNU Lesser General Public License v3.0
20 stars 23 forks source link

Fix buffer overflow causing CI on macos to fail #102

Closed lefessan closed 1 year ago

lefessan commented 1 year ago

The bug was triggered by the longgy.cob test that was added for caret, containing a very long identitier.

GitMensch commented 1 year ago

Thanks for checking.

Interestingly running the test with memcheck ./pre-inst-env valgrind --vgdb-error=1 cobc/.libs/cobc -fdiagnostics-plain-output -Wno-others -fdiagnostics-show-caret longgy.cob does not raise an error (it normally does in this case)

Looking at this function (actually its callers) I think it does too much and not enough. Can you please adjust it as follows?

This leads to a cleaner implementation, minimal speedup and the memory fix.

codecov-commenter commented 1 year ago

Codecov Report

Merging #102 (82eeb09) into gcos4gnucobol-3.x (8a0796d) will increase coverage by 0.13%. The diff coverage is 70.02%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                  Coverage Diff                  @@
##           gcos4gnucobol-3.x     #102      +/-   ##
=====================================================
+ Coverage              65.06%   65.19%   +0.13%     
=====================================================
  Files                     31       31              
  Lines                  58179    58323     +144     
  Branches               15323    15353      +30     
=====================================================
+ Hits                   37852    38025     +173     
+ Misses                 14406    14373      -33     
- Partials                5921     5925       +4     
Impacted Files Coverage Δ
libcob/common.c 56.60% <ø> (ø)
cobc/parser.y 68.69% <22.72%> (+0.13%) :arrow_up:
cobc/config.c 80.81% <41.66%> (-1.24%) :arrow_down:
cobc/scanner.l 75.83% <50.00%> (ø)
cobc/pplex.l 72.47% <65.71%> (+0.15%) :arrow_up:
libcob/fileio.c 57.70% <66.20%> (+0.70%) :arrow_up:
cobc/codegen.c 75.25% <69.23%> (+0.42%) :arrow_up:
cobc/typeck.c 64.98% <79.48%> (+0.09%) :arrow_up:
cobc/error.c 74.43% <83.82%> (+0.89%) :arrow_up:
libcob/numeric.c 79.84% <91.30%> (-0.35%) :arrow_down:
... and 3 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

GitMensch commented 1 year ago

I've just ran the result in Appveyor MacOS (took a while to fix the old brew installations). And guess what: this new test is the only one that is failing... after the changes.

build log testsuite.log

but that's because of the sed invocation, which does not work:

25. used_binaries.at:923: testing cobc diagnostics show caret ...
./used_binaries.at:942: $COBC -Wall -fsyntax-only prog.cob
./used_binaries.at:947: $COBC -fdiagnostics-show-caret -fdiagnostics-show-line-numbers prog.cob
./used_binaries.at:967: $COBC -fdiagnostics-show-caret prog.cob
./used_binaries.at:988: sed -e 's/DIVISION\./DIVISION  \.\t  \t  /' prog.cob > progsp.cob
./used_binaries.at:990: $COBC -Wno-others -fdiagnostics-show-caret progsp.cob
--- -   2023-06-28 09:08:42.000000000 -0500
+++ /Users/appveyor/projects/gnucobol-3-x-macos/tests/testsuite.dir/at-groups/25/stderr 2023-06-28 09:08:42.000000000 -0500
@@ -2,6 +2,11 @@
           WORKING-STORAGE  SECTION.
           01 TEST-VAR PIC 9(2) VALUE 'A'.
  >        COPY 'CRUD.CPY'.
-          PROCEDURE        DIVISION  .
+          PROCEDURE        DIVISION  .t  t
               DISPLAY TEST-VAR NO ADVANCING
+progsp.cob:2: error: syntax error, unexpected Identifier, expecting FUNCTION-ID or PROGRAM-ID
+   
+ >        IDENTIFICATION   DIVISION  .t  t
+          PROGRAM-ID.      prog.
+          DATA             DIVISION  .t  t

25. used_binaries.at:923: 25. cobc diagnostics show caret (used_binaries.at:923): FAILED (used_binaries.at:990)

:-/

... in any case it should be $SED, not sed.

ChatGPT says that for Darwin we'd need to use sed -e $'s/DIVISION\./DIVISION \.\t \t /' prog.cob > progsp.cob - not sure if this is correct (and if it would work everywhere else), at least it is "sure" about that:

The $'...' syntax for interpreting escape sequences is supported by most modern Unix-like systems, including GNU/Linux distributions and Solaris. It allows you to use escape sequences like \t to represent a tab character within the sed command.

Hm...

GitMensch commented 1 year ago

The easiest option is likely to only run the test once and in the testsuite source already have those tab + spaces included, followed by #, which is then not $SED but tr -d removed... Done that way, now also the previously failing (old) MacOS environments work again on Appveyor.