This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix UB in diagnostic.c
- From: Manuel López-Ibáñez <lopezibanez at gmail dot com>
- To: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Cc: Marek Polacek <polacek at redhat dot com>, Dodji Seketeli <dodji at redhat dot com>
- Date: Wed, 13 Aug 2014 21:03:37 +0200
- Subject: Re: [PATCH] Fix UB in diagnostic.c
- Authentication-results: sourceware.org; auth=none
> This should fix an undefined behavior in diagnostics.c.
> Under certain circumstances, max_width is (INT_MAX - 1),
> and right_margin is -4 -> the subtraction overflows.
> Changing the types to unsigned would involve changing
> much more code than just one cast.
> BTW, the diagnostics we output for the testcase in the PR
> is crap - but I'm not fixing it now.
I don't think this is the right fix. The problem is that we are trying
to print the caret in a column that is larger than the line_width. We
do this because the file given by the line directive has nothing to do
with the actual code we are parsing. I think in that case it is ok to
just give up and not print a caret. So something like:
@@ -300,11 +299,11 @@ diagnostic_show_locus (diagnostic_contex
context->last_location = diagnostic->location;
s = expand_location_to_spelling_point (diagnostic->location);
line = location_get_source_line (s, &line_width);
- if (line == NULL)
+ if (line == NULL || s.column > line_width)
max_width = context->caret_max_width;
line = adjust_line (line, line_width, max_width, &(s.column));
Nonetheless, perhaps in addition adjust_line should have
gcc_checking_assert(line_width >= column).
Another alternative is for location_get_source_line to check this and
return NULL in that case (since that location cannot belong to that
source line). But perhaps such behavior might be useful in other
situations (wrong column info but the file and line are correct).