Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location
On 05/29/2018 11:25 AM, Mike Gulick wrote: > On 03/04/2018 02:27 PM, Mike Gulick wrote: >> >> >> On 02/09/2018 05:54 PM, Mike Gulick wrote: >>> Hi David, >>> >>> Here is a new version of the linemap patch (see my earlier emails for an >>> updated >>> version of the test code). >> >> >> >> Hi David, >> >> Just wondering if you have had a chance to look at these updated patches >> yet. Let me know if you have any questions I can answer, or if there is >> anything you would like me to do that would make reviewing them easier >> (reposting, rebasing, refactoring the bug fix from the diagnostics >> change in the last patch). >> >> The most recent postings are: >> Bug fix patch: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00557.html >> Test case: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01993.html >> >> Thanks, >> Mike >> > > Hi David, > > Now that gcc 8.1 is out the door, would you have any time to review these > patches? I re-tested them after rebasing on the latest git master, and > everything still behaved as expected. I can post the rebased patches if you > would like, but it was a trivial merge with no conflicts. > > Thanks, > Mike > I'd still like to get this bug fixed. I don't think I need the [RFC] tag any more on these patches. Should I post a new thread to this list to get the ball rolling again?
Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location
On 02/09/2018 05:54 PM, Mike Gulick wrote: > Hi David, > > Here is a new version of the linemap patch (see my earlier emails for an > updated > version of the test code). Hi David, Just wondering if you have had a chance to look at these updated patches yet. Let me know if you have any questions I can answer, or if there is anything you would like me to do that would make reviewing them easier (reposting, rebasing, refactoring the bug fix from the diagnostics change in the last patch). The most recent postings are: Bug fix patch: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00557.html Test case: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01993.html Thanks, Mike
Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location
Hi David,
Here is a new version of the linemap patch (see my earlier emails for an updated
version of the test code). I spent several days poring over the line map code,
and I think I understand it a little better now. I also discovered
-fdump-internal-locations, and it was a big help. If I use this switch with the
code from the test I posted with an initial offset of 0x6001, I get the
following (I have removed some unnecessary fields for brevity):
ORDINARY MAP: 21
source_location interval: 1610612805 <= loc < 1610612809
file: location-overflow-test-pr83173.c
starting at line: 3
reason: 2 (LC_RENAME)
location-overflow-test-pr83173.c: 3|loc:1610612805| { dg-do preprocess }
location-overflow-test-pr83173.c: 4|loc:1610612806|*/
location-overflow-test-pr83173.c: 5|loc:1610612807|
location-overflow-test-pr83173.c: 6|loc:1610612808|#include
"location-overflow-test-pr83173.h"
ORDINARY MAP: 22
source_location interval: 1610612809 <= loc < 1610612810
file: location-overflow-test-pr83173.h
starting at line: 1
reason: 0 (LC_ENTER)
location-overflow-test-pr83173.h: 1|loc:1610612809|#include
"location-overflow-test-pr83173-1.h"
ORDINARY MAP: 23
source_location interval: 1610612810 <= loc < 1610612812
file: location-overflow-test-pr83173-1.h
starting at line: 1
reason: 0 (LC_ENTER)
location-overflow-test-pr83173-1.h: 1|loc:1610612810|#pragma once
location-overflow-test-pr83173-1.h: 2|loc:1610612811|#define
LOCATION_OVERFLOW_TEST_PR83173_1_H
ORDINARY MAP: 24
source_location interval: 1610612812 <= loc < 1610612812
file: location-overflow-test-pr83173.h
starting at line: 2
reason: 1 (LC_LEAVE)
ORDINARY MAP: 25
source_location interval: 1610612812 <= loc < 1610612814
file: location-overflow-test-pr83173-2.h
starting at line: 1
reason: 0 (LC_ENTER)
location-overflow-test-pr83173-2.h: 1|loc:1610612812|#pragma once
location-overflow-test-pr83173-2.h: 2|loc:1610612813|#define
LOCATION_OVERFLOW_TEST_PR83173_2_H
ORDINARY MAP: 26
source_location interval: 1610612814 <= loc < 1610612815
file: location-overflow-test-pr83173.h
starting at line: 2
reason: 1 (LC_LEAVE)
location-overflow-test-pr83173.h: 2|loc:1610612814|#include
"location-overflow-test-pr83173-2.h"
ORDINARY MAP: 27
source_location interval: 1610612815 <= loc < 1610612823
file: location-overflow-test-pr83173.c
starting at line: 7
reason: 1 (LC_LEAVE)
ORDINARY MAP: 28
source_location interval: 1610612823 <= loc < 1610612825
file: location-overflow-test-pr83173.c
starting at line: 15
reason: 2 (LC_RENAME)
Notice that map 24 and 25 have the same start_location. This is because
line_table->highest_location is being decremented when it should not be. This
new patch uses a more robust method to check whether the source line that
highest_location refers to is greater than loc (which is the source_location of
the #include). Instead of comparing the source_location values directly, I use
linemap_get_expansion_line to map the source location to a line number, and then
compare the line numbers. This handles the case that loc is an adhoc location.
I noticed that after updating the filenames in the test to be
'location-overflow-test-pr83173-1.h', the source locations passed into
_cpp_stack_include were adhoc locations when I wasn't using the location offset
plugin. This is because these filenames are now 34 characters long, which is
longer than the 5 range bits that an ordinary source_location allows.
I spent a lot of time comparing the -fdump-internal-locations output when this
patch is in use to the unpatched version. The only differences occur when a
#include is the last line in the file. For the case where the source location >
LINE_MAP_MAX_SOURCE_LOCATION, the ordinary map 25 (see above) now starts at
1610612813 instead of 1610612812. For the case where the source location <
LINE_MAP_MAX_LOCATION_WITH_COLS, the start location of that map has increased by
32 (which is effectively one column). For the case where the source location is
between LINE_MAP_MAX_LOCATION_WITH_COLS and LINE_MAP_MAX_SOURCE_LOCATION (so
that there are no range bits), the output was unchanged. I can post these
location map dumps if you would like to see them.
There are no regressions in the gcc test suite from this patch.
The other addition in this version if the patch is a small improvement to the
-fdump-internal-locations output to include the reason and included_from fields
from the line_map_ordinary data structure. I aos fixed an issue with the output
alignment. I found this useful when evaluating these changes.
Thanks,
Mike
From e3f23f66ad18598fc21a9223b7f29572a14d1588 Mon Sep 17 00:00:00 2001
From: Mike Gulick
Date: Fri, 1 Dec 2017 09:43:22 -0500
Subject: [PATCH v2] PR preprocessor/83173: Additional check before
decrementing highest_location
2017-12-01 Mike Gulick
PR preprocessor/83173
* libcpp/files.c (_cpp_stack_include): Check if
