There is one use in instructionAPI/src/ArchSpecificFormatters.C that's been there since 2017 and Compiler Explorer <https://godbolt.org/z/Yf5AUt> says it works, so we should be good on 4.8.4 compatibility. It's also building on targeted platforms successfully <https://bottle.cs.wisc.edu/branch/PR627>. As long as it works on your end, we can merge it.
Thanks. - Tim On Wed, Aug 7, 2019 at 4:28 PM Mark W. Krentel <kren...@rice.edu> wrote: > Ok, I'll test it. One quick thing you could do now is scan the code > and see if std::to_string() is something you already use or not. > > If you're already using it in several places, then probably it won't > be a problem. > > --Mark > > > > On 08/07/19 14:51, Tim Haines wrote: > > Hi, Mark. > > I created a PR <https://github.com/dyninst/dyninst/pull/627> to fix this, > could you test on your machines? > > >Before you commit anything, test it with gcc 4.8.x. I'm not sure if the > 4.8 compiler implements 100% of C++11. > I don't have any systems with a gcc that old. If you do, could you test > there? > > Thanks. > > - Tim > > > > On Wed, Aug 7, 2019 at 2:03 PM Mark W. Krentel <kren...@rice.edu> wrote: > >> Tim, >> >> Based on the contents of the <inttypes.h> file, I don't see where this >> would be limited to 32-bit. >> >> I have seen this on more than just power8. With RH 6.x, it's "broken" >> on both x86_64 and power7, both with glibc-headers-2.12. With RH 7.x, >> it's broken on power8 but not x86_64, both with glibc-headers-2.17. >> >> Where "broken" means that <inttypes.h> won't define PRIx64 without >> __STDC_FORMAT_MACROS. >> >> I guess this is old code and the #if was removed at some point, but >> removed on x86 before ppc. But that may be moot now. >> >> One thing about std::to_string() and C++11. Before you commit >> anything, test it with gcc 4.8.x. I'm not sure if the 4.8 compiler >> implements 100% of C++11. >> >> --Mark >> >> >> >> On 08/07/19 13:33, Tim Haines wrote: >> >> Hi, Mark. >> >> I built the head of master on all of our usual test platforms, and only >> found this >> isshttps://github.com/dyninst/dyninst/blob/e22e663ed73bfe507fcd56b72ee13043634ae4c1/instructionAPI/h/Result.hue >> <https://bottle.cs.wisc.edu/commit/e22e663ed73bfe507fcd56b72ee13043634ae4c1> >> on Power8 with gcc-7.3.1 (Power9 with gcc-7.3.1 was fine, so that's fun!). >> >> Honestly, the PR* macros aren't the correct solution to this problem. I >> probably should have forced a code change before merging this, but it had >> already been tested by the person who submitted the PR, and it was blocking >> their build cycle. The correct solution is to use to use std::to_string >> introduced in C++11. I'll make a PR for that and re-test. >> >> Thanks. >> >> - Tim >> >> >> On Tue, Aug 6, 2019 at 3:35 PM Mark W. Krentel <kren...@rice.edu> wrote: >> >>> One of my cron jobs tripped an alarm for Dyninst master on powerpc >>> this morning. >>> >>> This one is bizarre, but the last commit about the PRIx64 macros >>> breaks the build on some systems, depending on the exact version of >>> the <inttypes.h> header file. >>> >>> Here is a unit test that reproduces the problem. This is a snippet >>> from instructionAPI/h/Result.h, near line 453. >>> >>> print.cpp: >>> >>> #include <sstream> >>> #include <string.h> >>> #include <inttypes.h> >>> >>> int >>> main(int argc, char **argv) >>> { >>> char hex[20]; >>> int64_t val = 123; >>> >>> snprintf(hex, 20, "%" PRIx64, val); >>> >>> printf("ans = %s\n", hex); >>> return 0; >>> } >>> >>> On one power8 system with glibc-headers-2.17-157.el7.ppc64le, >>> >>> $ g++ -g -O -std=c++11 print.cpp >>> >>> print.cpp: In function 'int main(int, char**)': >>> print.cpp:11:27: error: expected ')' before 'PRIx64' >>> snprintf(hex, 20, "%" PRIx64, val); >>> ^~~~~~ >>> >>> Building Dyninst, I get the same error in Result.h near line 453. >>> >>> I had to diff <inttypes.h> across systems to figure out what's going >>> on. The powerpc version has the following. The x86_64 version does >>> not have the #if on lines 44-46. >>> >>> 44 /* The ISO C99 standard specifies that these macros must only be >>> 45 defined if explicitly requested. */ >>> 46 #if !defined __cplusplus || defined __STDC_FORMAT_MACROS >>> 47 >>> 48 # if __WORDSIZE == 64 >>> 49 # define __PRI64_PREFIX "l" >>> 50 # define __PRIPTR_PREFIX "l" >>> 51 # else >>> 52 # define __PRI64_PREFIX "ll" >>> 53 # define __PRIPTR_PREFIX >>> 54 # endif >>> >>> So, on powerpc, things like PRIx64 are either not defined or else not >>> defined the way you want them. >>> >>> I'm not sure if this is a glibc-headers version issue, or an x86 vs >>> ppc issue. It seems to be more common on powerpc, but older RH 6.x >>> x86_64 systems also have the same problem. >>> >>> Anyway, I'll let you decide the best and most portable way of handling >>> this. Maybe you just want to define __STDC_FORMAT_MACROS and test >>> that it doesn't break anything else. >>> >>> Blech. My kingdom for portability! >>> >>> --Mark >>> >>> >>> _______________________________________________ >>> Dyninst-api mailing list >>> Dyninst-api@cs.wisc.edu >>> https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api >>> >> >> >
_______________________________________________ Dyninst-api mailing list Dyninst-api@cs.wisc.edu https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api
