Skip to content

Commit

Permalink
Step 5: use uintptr_t for 257-indexed arrays in inftrees.c
Browse files Browse the repository at this point in the history
  • Loading branch information
pascal-cuoq committed Aug 1, 2016
1 parent d7cde11 commit 46c3f2f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
24 changes: 12 additions & 12 deletions inftrees.c
Expand Up @@ -2,7 +2,7 @@
* Copyright (C) 1995-2013 Mark Adler
* For conditions of distribution and use, see copyright notice in zlib.h
*/

#include <stdint.h>
#include "zutil.h"
#include "inftrees.h"

Expand Down Expand Up @@ -52,8 +52,8 @@ unsigned short FAR *work;
unsigned mask; /* mask for low root bits */
code here; /* table entry for duplication */
code FAR *next; /* next available space in table */
const unsigned short FAR *base; /* base value table to use */
const unsigned short FAR *extra; /* extra bits table to use */
uintptr_t base; /* base value table to use */
uintptr_t extra; /* extra bits table to use */
int end; /* use base and extra for symbol > end */
unsigned short count[MAXBITS+1]; /* number of codes of each length */
unsigned short offs[MAXBITS+1]; /* offsets in table for each length */
Expand Down Expand Up @@ -180,19 +180,19 @@ unsigned short FAR *work;
/* set up for code type */
switch (type) {
case CODES:
base = extra = work; /* dummy value--not used */
base = extra = (uintptr_t) work; /* dummy value--not used */
end = 19;
break;
case LENS:
base = lbase;
base -= 257;
extra = lext;
extra -= 257;
base = (uintptr_t) lbase - 257 * sizeof(short);

extra = (uintptr_t) lext - 257 * sizeof(short);

end = 256;
break;
default: /* DISTS */
base = dbase;
extra = dext;
base = (uintptr_t) dbase;
extra = (uintptr_t) dext;
end = -1;
}

Expand Down Expand Up @@ -221,8 +221,8 @@ unsigned short FAR *work;
here.val = work[sym];
}
else if ((int)(work[sym]) > end) {
here.op = (unsigned char)(extra[work[sym]]);
here.val = base[work[sym]];
here.op = (unsigned char)*(short*)(extra + sizeof(short)*work[sym]);
here.val = *(short*)(base + sizeof(short)*work[sym]);
}
else {
here.op = (unsigned char)(32 + 64); /* end of block */
Expand Down
10 changes: 10 additions & 0 deletions tis-interpreter-tutorial.md
Expand Up @@ -107,3 +107,13 @@ inftrees.c:188:[kernel] warning: pointer arithmetic: assert \inside_object(base-
main
[value] Stopping at nth alarm
```

---

STEP 5

The `inftrees.c` warning is easy to confirm, but a bit unpleasant to deal with: the code [is making](https://github.com/pascal-cuoq/zlib-fork/blob/6efef49d0ffd78f82e1ae7127cc3819d64ebc219/inftrees.c#L187-L188) the pointer `base` point to the first element of an array `lbase`, and then subtracts 257 from it using pointer arithmetic. The intention is to make a sort of 257-indexed array, but these aren't any more allowed than [1-indexed arrays made by subtracting one](http://blog.regehr.org/archives/1292) are.

An implementation-defined solution for compilation platforms with a flat address space is to use only `uintptr_t` to refer to addresses outside `lbase`. This solution may still cause issues on platforms where the mapping between pointers and integers is more complicated than the one-to-one correspondance of flat address spaces, but let's face it, if you are targeting a platform without a flat address space, you should not be playing with out-of-bounds pointers either.

There may exist a cleaner solution than the one implemented in this commit for pointers `base` and `extra`, but I do not immediately see it.

0 comments on commit 46c3f2f

Please sign in to comment.