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 GulickDate: 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