Fix ASAN errors due adding offset to nullptr

[parmeet]

This PR fixes the error when Address Sanitization is enabled. The offset is added to nullptr which gives following runtime error:

applying non-zero offset 4 to null pointer

The PR checks whether the pointer is nullptr, and if so do not add offset to it.

[stevengj]

Maybe cleaner to combine into a single call with a dst ? ... argument

Suggested change

	if (!dst){
	written += utf8proc_decompose_char(entry_cp, dst,
	(bufsize > written) ? (bufsize - written) : 0, options,
	last_boundclass);
	}else{
	written += utf8proc_decompose_char(entry_cp, dst+written,
	(bufsize > written) ? (bufsize - written) : 0, options,
	last_boundclass);
	}
	written += utf8proc_decompose_char(entry_cp,
	dst ? dst+written : dst,
	(bufsize > written) ? (bufsize - written) : 0, options,
	last_boundclass);

[stevengj]

(Note that the ASAN warning is harmless in this case, since utf8proc_decompose_char doesn't write anything if bufsize is 0 as will be the case here. But we might as well silence ASAN here.)

[parmeet]

(Note that the ASAN warning is harmless in this case, since utf8proc_decompose_char doesn't write anything if bufsize is 0 as will be the case here. But we might as well silence ASAN here.)

Actually it's not just about the warning. I have noticed runtime error caused due to this that makes the program crash.

[stevengj]

I have noticed runtime error caused due to this that makes the program crash.

Could you elaborate? I don't see any if (dst == NULL) checks or similar, so I don't see what preserving NULL could accomplish (other than silencing ASAN).

[parmeet]

@stevengj This is my rough understanding what's going on:

When utf8proc_decompose_custom is called here

utf8proc/utf8proc.c

Line 736 in 63f31c9

result = utf8proc_decompose_custom(str, strlen, NULL, 0, options, custom_func, custom_data);

inside utf8proc_map_custom with buffer == NULL, and wpos is incremented inside the while loop, it is adding wpos (>0) to buffer which is NULL. This give run-time error applying non-zero offset 4 to null pointer. By incorporating changes in this PR, I could get rid of these run-time errors.

[stevengj]

That sounds like a runtime error caused by ASAN. Applying a nonzero offset to a null pointer is perfectly safe if you never dereference the pointer.

[xaizek]

That sounds like a runtime error caused by ASAN. Applying a nonzero offset to a null pointer is perfectly safe if you never dereference the pointer.

Answers to this question suggest that it's a UB. Although I'm not sure how come NULL pointer is "a pointer into, or just beyond, an array object", but clang generates an undefined behaviour error for this which suggest it might be a valid interpretation.

[dzfrias]

Running into similar issues while writing a Zig compilation for utf8proc. Having to compile with -fno-sanitize=pointer-overflow or else a trap occurs. Although it's technically not a relevant warning, I think compiling with sanitization (the default for Zig) is common enough that it makes sense to work around these null pointer offsets runtime errors.

There are other places in the library that use null pointer offsets. If this PR is approved, I can work on a follow up.

[eschnett]

Memory access violations in C string processing libraries are a major source of bugs. Thus tools like ASAN and MSAN are very important. I think we should merge this PR and avoid all other similar C standard violations.

[stevengj]

Fair enough, it's good to allow users to use these tools, and we're going to get bombarded with complaints if we violate their requirements (even if our violations are harmless in practice).