3

I'm trying to steal two LSB bits from address and I have written the following inline functions. I want to confirm if this works on both 64 and 32 bit machines.

I used stealing bits from a pointer post as reference.

I get a weird bug where getAddress() returns 0x100

0x1 is fine as setFlag1(NULL) will return this address

0x2 is fine as setFlag2(NULL) will return this address

0x11 is also fine as setFlag1(NULL) and setFlag2(NULL) will return this address

But I'm not sure what combination will return 0x100

#define UINTPTR_MAX_XOR_WITH_1 (uintptr_t) (UINTPTR_MAX ^ 0x1)
#define UINTPTR_MAX_XOR_WITH_2 (uintptr_t) (UINTPTR_MAX ^ 0x2)
#define UINTPTR_MAX_XOR_WITH_3 (uintptr_t) (UINTPTR_MAX ^ 0x3)

struct node
{
    unsigned long key;
    struct node* lChild;    //format <address,flag2,flag1>
    struct node* rChild;    //format <address,flag2,flag1>
};

static inline struct node* getAddress(struct node* p)
{
    return (struct node*)((uintptr_t) p & UINTPTR_MAX_XOR_WITH_3);
}

static inline bool isFlag2(struct node* p)
{
    return (uintptr_t) p & 0x2;
}

static inline bool isFlag1(struct node* p)
{
    return (uintptr_t) p & 0x1;
}

static inline struct node* setFlag1(struct node* p)
{
    return((struct node*) (((uintptr_t) p & UINTPTR_MAX_XOR_WITH_1) | 0x1));
}

static inline struct node* unsetFlag1(struct node* p)
{
    return((struct node*) (((uintptr_t) p & UINTPTR_MAX_XOR_WITH_1)));
}

static inline struct node* setFlag2(struct node* p)
{
    return((struct node*) (((uintptr_t) p & UINTPTR_MAX_XOR_WITH_2) | 0x2));
}
Community
  • 1
  • 1
arunmoezhi
  • 3,082
  • 6
  • 35
  • 54
  • 3
    Can you show some code that actually does return `0x100`? – Useless Apr 01 '14 at 18:15
  • Why are you using `UL` constants? – Filipe Gonçalves Apr 01 '14 at 18:15
  • @Useless: I generate a tree using the `node` structure and a child of a node is at `0x100`. `gdb` told me that I get a segfault because I'm trying to access key at location `0x100` – arunmoezhi Apr 01 '14 at 18:19
  • @FilipeGonçalves: I did use `0x2` `0x1` and `0x3` initially. Had the same issue. Then I tried `0x2UL` and the issue still existed – arunmoezhi Apr 01 '14 at 18:20
  • You probably already pass in an invalid pointer to `getAddress()`. Did you test this? – alk Apr 01 '14 at 18:22
  • 5
    0x100 cannot be a combination of your flags, and it's unlikely to be a valid address on most modern platforms, so your problem began when you got that address from somewhere. – Useless Apr 01 '14 at 18:26
  • @alk: I did check that. When I get a segfault, the `parent` pointer is valid and has a valid key. But the `child` pointer has this address. So this is the fist time an invalid address is sent to `getAddress()`. I know I should provide my whole code to get full help. But I'm not supposed to share the code. – arunmoezhi Apr 01 '14 at 18:27
  • @Useless: That makes sense. Let me debug my actual algorithm. I wanted to make sure I did not mess up with `getAddress()` and `setFlags()` function which does the bit masking/stealing – arunmoezhi Apr 01 '14 at 18:30
  • In the post you linked they are stealing the MSBs not the LSBs as you are attempting to do. But I suppose maybe you don't need them if you're doing aligned allocations... what is your scenario in this regard that should allow this to work? Perhaps you mean to be stealing the MSBs? Also, I have to say, if you find yourself wanting to pack data into a pointer, it's probably worth considering if you can use a smaller sized index (say a 16-bit short) to achieve the same thing, and then packing it with a struct with bit fields or 1-byte aligned members which containing your extra data. – Apriori Apr 01 '14 at 19:20
  • @Apriori: In the linked post they are stealing from the LSB and I'm doing the same – arunmoezhi Apr 01 '14 at 20:17
  • 1
    Put some parentheses around th whole macro expansion, ugh! – Kaz Apr 01 '14 at 21:10
  • @arunmoezhi, the [chosen answer](http://stackoverflow.com/a/19389323/3100771) states: "You need only 30 bits to address the entire memory space, so the upper 2 bits are unused. You can use these upper two bits for your own purposes." That would be the most significant bits (MSBs). Unless somewhere else in the post talks about using the LSBs. – Apriori Apr 01 '14 at 21:13
  • It's not the cause of the problem, but your `set` and `unset` functions are over-complicated. There's no point in or-ing with zero in the `unset` functions (it's a no-op) and in the `set` functions it's pointless to clear the bit before setting it - just set it. – nobody Apr 01 '14 at 21:35
  • @Apriori: I used the other answer which has the code snippet – arunmoezhi Apr 01 '14 at 22:20
  • @Kaz: I don't understand why I need an extra parentheses – arunmoezhi Apr 01 '14 at 22:22
  • @AndrewMedico: Ya I get it. I should remove the `ORing` with zero in my `unset` function – arunmoezhi Apr 01 '14 at 22:25
  • 1
    @arunoehzi Macros that produce C expressions should almost always produce fully parenthesized C expressions, except when they produce function calls: `#define foo(a) bar(a)`, or single tokens `#define x y`. Your macros produce (type) (expr). The (type) cast is a unary operator which has a lower precedence than postfix operators. It's almost certainly not an issue in your program, but I had to think about that. If the parens are there, you don't have to think about it: you know that the macro expansion is a proper syntactic unit that won't be torn apart by precedence in a neighboring operator. – Kaz Apr 01 '14 at 22:36
  • @kaz: how should I modify my macro. I'm sure it is not the cause for the bug. But as you said it is good to have unambiguous code – arunmoezhi Apr 01 '14 at 22:48
  • `#define UINTPTR_MAX_XOR_WITH_3 ((uintptr_t) (UINTPTR_MAX ^ 0x3))` – Kaz Apr 01 '14 at 23:04
  • @Kaz If `UINTPTR_MAX` 2 LSBits are not both 1, `UINTPTR_MAX ^ 0x3` does not return a pointer with its 2 LSBits cleared. Although certainly, in _most_ platforms, `UINTPTR_MAX` 2 LSBits are both 1. {Edit} I see your comment is emphasizing adding the outer parens. – chux - Reinstate Monica Apr 02 '14 at 13:25
  • @chux Since UINTPTR_MAX is the highest value of an unsigned type, it must be a Mersenne number: all 1 bits. – Kaz Apr 02 '14 at 14:35
  • @Kaz Certainly a `uintptr_t`, as an unsigned integer, must be a Mersenne number. I thought of `UINTPTR_MAX` as the maximum value of an integer holding a _valid_ pointer. I now see it is the "maximum value of pointer-holding unsigned integer type". I stand corrected. – chux - Reinstate Monica Apr 02 '14 at 14:50
  • @Kaz Your comment caused me to re-think `SIZE_MAX` also. Again in error, I thought of `SIZE_MAX` as the largest number, for a given system, in which `malloc()` would ever respond with a non-NULL value. I now see it instead of the largest value representable in `size_t`. `SIZE_MAX` meets or exceeds a given system's `malloc()`'s limit. LSNED – chux - Reinstate Monica Apr 02 '14 at 15:20
  • 2
    @ALL: Thanks for the comments and suggestions. I was able to fix the bug. I was dereferencing a pointer to `struct node` without calling `getAddress` function. That caused an invalid memory access. – arunmoezhi Apr 02 '14 at 17:14

2 Answers2

3

The bug is probably not related to this tagging scheme at all.

It is possible for getAddress to return the address 0x100: you just have to feed it any of the inputs 0x100 through 0x103.

All that your getAddress function does is strip out the two least significant bits in order to recover an aligned pointer. Since 0x100 is clear in its two least significant bits, getAddress is doing its job.

GIGO: garbage in, garbage out!

Add some tracing to print the values that are going into getAddress.

Addresses like 0x100 sometimes result when a null pointer is dereferenced to obtain the address of a structure member: null_pointer->something_at_offset_256. And of course, in general, they can arise in any number of ways.

Kaz
  • 55,781
  • 9
  • 100
  • 149
2

The various function depend on UINTPTR_MAX having its 2 LSBits set. Re-coded to eliminate that dependency.

Some simplifications.

Performed comparison for bool return.

Be sure these functions, when called, were properly declared or prototyped. Else the return value is assumed to be int and could explain the 0x100 return.

The problem may still lie elsewhere. Are you expecting calls like setFlag1(p) to set p's bit (which code does not do) or return a pointer with the bit set (which code does)?

static inline struct node* getAddress(struct node* p) {
  return (struct node*)((uintptr_t) p & ~((uintptr_t) 3));
}

static inline bool isFlag1(struct node* p) {
  return ((uintptr_t) p & 1) != 0;
}

static inline bool isFlag2(struct node* p) {
  return ((uintptr_t) p & 2) != 0;
}

static inline struct node* setFlag1(struct node* p) {
  return (struct node*) ((uintptr_t) p | 1);
}

static inline struct node* unsetFlag1(struct node* p) {
  return (struct node*) ((uintptr_t) p & ~((uintptr_t) 1));
}

static inline struct node* setFlag2(struct node* p)
  return (struct node*) ((uintptr_t) p | 2);
}

static inline struct node* unsetFlag2(struct node* p) {
  return (struct node*) ((uintptr_t) p & ~((uintptr_t) 2));
}

[Edit] In review, it is certain that UINTPTR_MAX 2 LSBits must be 1. See @Kaz comment above.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • `setFlag1(p)` will return the pointer which has the bit set. I do the assignment in the calling function – arunmoezhi Apr 01 '14 at 23:20
  • any reason why you use `0x1` at `isFlag1` and just `1` at `setFlag1`. I assume it doesn't make a difference. Also why do you typecast `1` to `uintptr_t` in `unsetFlag1` and not it `setFlag1`. Same for `2` – arunmoezhi Apr 01 '14 at 23:46
  • `Be sure these functions, when called, were properly declared or prototyped. Else the return value is assumed to be int and could explain the 0x100 return.` Why do you say this. The return values of these functions are `struct node*`. In the calling function if I dont type cast them, how will they get mapped to an `int`?. – arunmoezhi Apr 02 '14 at 00:50
  • Lets say I have a pointer `p`. To get the `lChild` pointer, I do `getAddress(p)->lChild`. Here `getAddress(p)` returns a `struct node*` on which I invoke `->lChild`. What can go wrong here? – arunmoezhi Apr 02 '14 at 00:57
  • @OP 1a) No reason for `0x1` vs `1`. 1b) When or'ing the 1, the integer size of the 1 is not important - all more significant bits are 0. When and'ing with ~1, it is important that the size of `1` is correct before applying the `~` to make certain the integer width is at least as long as `uintptr_t` 2) If you call the function without an earlier prototype or definition, the compiler assumes a return value of `int`. 3) `getAddress(p)->lChild` looks good. – chux - Reinstate Monica Apr 02 '14 at 02:23