Skip to content

Fix crashes in mgd77magref with a numeric -D - #9270

Open
Esteban82 wants to merge 4 commits into
masterfrom
fix-mgd77magref-dst
Open

Esteban82 wants to merge 4 commits into
masterfrom
fix-mgd77magref-dst

Conversation

@Esteban82

Copy link
Copy Markdown
Member

Claude analyzed mgd77magref and found a few bugs around the Dst buffer that lead to crashes. This PR proposes the fix. The scripts below show them (on current master the first three segfault; the last one segfaults after the error for the bad segment).

  1. -D<number> segfaults: parse() wrote the value into CM4_D.dst before it was allocated (it is allocated later in the module body).
  2. -D-8 behaved as -D8: the leading - was skipped before atof.
  3. With several input records a numeric -D read past the end of its 1-element dst array.
  4. A segment outside the Dst time span freed dst in MGD77_cm4field, so the next single-record segment wrote into a NULL pointer.
    dst is now allocated at parse time and freed only in Free_Ctrl. Adds test/mgd77/magref_dst.sh.

Tested with:

export GMT_END_SHOW=off
echo "-30 45 0 2000.08700533" | gmt mgd77magref -A+y -Fxyz/3 -D8
echo "-30 45 0 2000.08700533" | gmt mgd77magref -A+y -Fxyz/3 -D-8
printf -- "-30 45 0 2000.087\n-30 45 0 2000.5\n-30 45 0 2001.2\n" | gmt mgd77magref -A+y -Fxyz/3 -D8
printf -- "> bad\n-30 45 0 1900.5\n-30 45 0 1900.6\n> good\n-30 45 0 2000.5\n" | gmt mgd77magref -A+y -Fxyz/3

Written with Claude Sonnet 5.5, reviewed with Claude Opus 5.5.

This is the output with this fix:

-8.91517189718  3.78735619674   10.5974519577
-15.5064972641  4.44507960625   19.2142882413
-8.88607779405  3.77783244797   10.6115537859
-6.01975730977  2.8274257026    17.2528952866
-8.80171128582  2.58243039681   12.9876225555
> bad
INTDST -- Error: T (15202; 15203) LIES OUTSIDE OF DST TABLE TIME SPAN [35839; 60675] -- ABORT
mgd77magref [ERROR]: this segment has a record generating an error.
Unfortunately, this means all other eventually good
records are also ignored. Fix the bad record and rerun the command.
> good
-3.84802225651  2.23032262522   14.4369055462

Esteban82 and others added 4 commits October 16, 2026 17:26
…d77magref

parse() stored the Dst value in a buffer that was only allocated later, and
skipped the leading minus sign when converting. With several input records a
numeric -D also read past the one-element dst array.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
MGD77_cm4field freed it on that error, so the next single-record segment
wrote into a NULL pointer. Free_Ctrl now owns the buffer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Esteban82
Esteban82 requested review from joa-quim and seisman October 17, 2026 19:43
@Esteban82 Esteban82 added bug Something isn't working add-changelog Add PR to the changelog labels Oct 17, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add-changelog Add PR to the changelog bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant