Bug 1402516

TLDR: the fix consists in replacing the following non portable code:

  CharT curr_char_ = sizeof (CharT) == 1 ? -128 : 0; 

by the portable code:

  CharT curr_char_ = (std::numeric_limits<CharT>::min)();

That change apparently causes other problems when wchar_t is signed, see

https://github.com/boostorg/spirit/commit/c480d6c7fcadf3cb5fbaad756ac370275a75e601

That commit only fixes one of the places that the fix for ticket 8291 changed. It would be nice to know what on earth that code is actually trying to do. Surely fixing it properly and portably can't be difficult.

The original change was https://github.com/boostorg/spirit/commit/2b4c331e70f1f8eaae33537df12c5744dd8e8b09

That code fills a lookup table mapping characters to discrete finite automaton states for the lexer. Depending on the token_._negated flag, it either maps all characters in the token's charset or all possible characters _except_ for those in the charset.

So iterating curr_char_ from std::numeric_limits<CharT>::min)() would actually be correct.

A proper upstream fix would then either cast the index into the lookup table (ptr_[static_cast<typename Traits::index_type>(curr_char_)]) to unsigned or shift it by -std::numeric_limits<CharT>::min)()... consistently everywhere the
the lookup table is used. A bit more involved.

The kludge from https://github.com/boostorg/spirit/commit/c480d6c7fcadf3cb5fbaad756ac370275a75e601
starts with curr_char_=0, which works correctly on platforms on which wchar_t is unsigned int. On platforms where it's signed, it should work, too, as long as we're processing only characters >=0... which on >=32 bit platforms should encompass all relevant ones.

Thanks! Now that bug 1404781 is fixed I'll have another try at applying those two patches and building a new package.

Scratch build with those two patches:
https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12254351

Any movement on this? In particular, is there a chance to get this async into 7.3 as work with one of our partners depends on this?

I have the patch ready to go, but making a change for rhel7 needs to be acked.

Did anyone test the scratch build I did?

I'm afraid I didn't get to test before the scratch build expired. I'm sorry, would you mind kicking off a build again?

Can test only on x86_64, not ppc64, though.

(In reply to Jonathan Wakely from comment #9)
> I have the patch ready to go, but making a change for rhel7 needs to be
> acked.
> 
> Did anyone test the scratch build I did?

Sorry I was on PTO, so I missed it and now the scratch build is gone. Could you rebuild it? I am going to test it with UHD.

Hi, thanks for offering to test. Any testing is useful, even if not on ppc64. There's a new scratch build at https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12533685

x86_64 RPMs are at:
https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12533691

and ppc64 at:
https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12533697

Thanks for checking them!

(In reply to Jonathan Wakely from comment #12)
> Hi, thanks for offering to test. Any testing is useful, even if not on
> ppc64. There's a new scratch build at
> https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12533685
> 
> x86_64 RPMs are at:
> https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12533691
> 
> and ppc64 at:
> https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=12533697
> 
> Thanks for checking them!

The scratch build works for me, the problem seems fixed, thanks.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:1973