4 hours ago, Finalspace said:

Hi there,

i previously had a function which can wait for a array of threads to finish - implemented in C99:

bool fpl__Win32ThreadWaitForMultiple(fplThreadHandle **threads, const size_t count, const fplTimeoutValue timeout) {
	for (size_t index = 0; index < count; ++index) {
		fplThreadHandle *thread = threads[index];
		if (thread == fpl_null) {
			return false;
		}
		
		// ... Check the thread
	}
	return true;
}

 

This was fine for a very long time, but now i encountered a case where my thread handle pointers are not following in order (Not contiguously), so i introduced a "stride" argument to account for that:

bool fpl__Win32ThreadWaitForMultiple(fplThreadHandle **threads, const size_t count, const size_t stride, const fplTimeoutValue timeout {
	for (size_t index = 0; index < count; ++index) {
		fplThreadHandle *thread = *(fplThreadHandle **)((uint8_t *)threads + index * stride);
		if (thread == fpl_null) {
			return false;
		}
		
		// ... Check the thread
	}
	return true;
}

 

This works fine unless you pass an invalid stride which results in either a invalid memory address or just garbage.

Now it comes the actual question -> Is there a way to detect such garbage memory addresses? Obviously a NULL check is not sufficient enough.

Visual studio reported a 0x21 as a write access violation...

 

I understand that i can detect any cases, but i would love to detect at least the most common cases.

 

I dont assume i can just check for "0x0000000000000021" on x64 isn´t it?

I mean you might be able to use VirtualQueryEx to found out if the address you are generating is valid but I would hardly think it's worth it.  You can also check for the last 3 bits being 0 (assuming a 64 bit machine) if you want to check for data alignment.

Also perhaps I'm missing something but I don't see the point of converting to a single byte granularity pointer, generating the address, and then converting back.   I would think stride has to be a multiple of the pointer size to work, so why not just pass in stride in in terms of that, multiply it by the index and use the regular array operator.  As near as I can tell to the only reason to do what you are doing is if fplThreadHandle is oddly aligned in memory.

Quote

This works fine unless you pass an invalid stride which results in either a invalid memory address or just garbage.

Garbage in -> Garbage out, that's kind of a key feature of C/C++ to make it fast

Quote

Now it comes the actual question -> Is there a way to detect such garbage memory addresses? Obviously a NULL check is not sufficient enough.

Yes and no... you can use virtual memory mapping and setting page protection, but this will only help for things that fit in a page (a common technique is to detect underflow by aligning it to the start of a page, and mapping the page use before it as protected... similarly aligning what you want to the end of the page and mapping the page after it as protected... but this requires 2 separate runs)

Then another common method is to use a stomp detector, but that wouldn't help you in your case because you're storing other data in between.

Thirdly you could look into Valgrind or any other similar checking tool.

But in key essence what you are doing here isn't really invalid... in C/C++ you are allowed to cast any memory address to anything... and it is up to you to make sure you adhere to the rules of what is there. (e.g. casting an atomic int to a 1 byte address, will cause invalid behavior too.)

So unfortunately this is PBKAC and there's not a whole lot of tools to fix that.

If someone gives you invalid inputs, feel free to invoke undefined behavior. There's nothing to fix.