Hello again, attached the patch. It declares inputBuf as unsigned so all bit shifts happen on unsigned values. ctest at least seems to be happy.
It does build without the casts as well but I am not completely sure about the language legalese behind this and hence left them in and also for explicitness. Proper fix would probably be to converted all of the LZW decoding to use unsigned values. Best regards, Adam Am 23.05.2018 um 21:24 schrieb Albert Astals Cid: > El dimecres, 23 de maig de 2018, a les 8:57:27 CEST, Adam Reichold va > escriure: >> Hello, >> >> maybe the simplest solution would to turn inputBuf into an unsigned int >> and convert to signed int after extracting the bits out of it? > > Yeah that sounds like a plan, could you try to produce a patch so i can run > it > through regtest? > > Cheers, > Albert > >> >> Best regards, >> Adam >> >> Am 23.05.2018 um 00:24 schrieb Albert Astals Cid: >>> poppler/Stream.cc | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> New commits: >>> commit 58e056c4b15f262b7715f8061d6885eb80044d0d >>> Author: Albert Astals Cid <[email protected]> >>> Date: Wed May 23 00:23:19 2018 +0200 >>> >>> Revert 31c3832b996acbf04ea833e304d7d21ac4533a57 >>> >>> So shifting left negative values is undefined behaviour according to >>> the >>> spec but if we don't do it we break, so we seem to be depending on >>> this >>> undefined behaviour, will try to figure out a better fix >>> >>> diff --git a/poppler/Stream.cc b/poppler/Stream.cc >>> index b6bfd838..4f075c12 100644 >>> --- a/poppler/Stream.cc >>> +++ b/poppler/Stream.cc >>> @@ -1445,9 +1445,7 @@ int LZWStream::getCode() { >>> >>> while (inputBits < nextBits) { >>> >>> if ((c = str->getChar()) == EOF) >>> >>> return EOF; >>> >>> - if (likely(inputBuf >= 0)) { >>> - inputBuf = (inputBuf << 8) | (c & 0xff); >>> - } >>> + inputBuf = (inputBuf << 8) | (c & 0xff); >>> >>> inputBits += 8; >>> >>> } >>> code = (inputBuf >> (inputBits - nextBits)) & ((1 << nextBits) - 1); >>> >>> _______________________________________________ >>> poppler mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/poppler > > > > >
diff --git a/poppler/Stream.cc b/poppler/Stream.cc
index 4f075c12..63c803dd 100644
--- a/poppler/Stream.cc
+++ b/poppler/Stream.cc
@@ -1445,10 +1445,10 @@ int LZWStream::getCode() {
while (inputBits < nextBits) {
if ((c = str->getChar()) == EOF)
return EOF;
- inputBuf = (inputBuf << 8) | (c & 0xff);
+ inputBuf = (inputBuf << 8) | static_cast<unsigned>(c & 0xff);
inputBits += 8;
}
- code = (inputBuf >> (inputBits - nextBits)) & ((1 << nextBits) - 1);
+ code = static_cast<signed>((inputBuf >> (inputBits - nextBits)) & ((1 << nextBits) - 1));
inputBits -= nextBits;
return code;
}
diff --git a/poppler/Stream.h b/poppler/Stream.h
index a3faccd9..dff7978d 100644
--- a/poppler/Stream.h
+++ b/poppler/Stream.h
@@ -823,7 +823,7 @@ private:
StreamPredictor *pred; // predictor
int early; // early parameter
GBool eof; // true if at eof
- int inputBuf; // input buffer
+ unsigned inputBuf; // input buffer
int inputBits; // number of bits in input buffer
struct { // decoding table
int length;
signature.asc
Description: OpenPGP digital signature
_______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
