duneroadrunner

السنة الماضيةdiscuss

So I don't write much C code these days, but I recently encountered strtol() again and am I mistaken or does the interface also violate const correctness? I mean it takes a const char* as the first parameter and then gives you back a (non-const) char* potentially pointing into the same string, right? Like, does strtol() get a pass because it's old, or is const correctness (still) not generally a concern of C programmers?

duneroadrunner

قبل 7 سنواتdiscuss

ArrayBufferBuilder isn't, but DOMArrayBuffer seems to be a GC managed type [1], right? And, before the patch, the DOMArrayBuffer held a "refcounting pointer" potentially targeting raw_data_'s reference counted ArrayBuffer, right? I don't see any immediately apparent use-after-free with this, so I assume raw_data_'s ArrayBuffer is being messed with elsewhere? As someone who worked on the code, do you have an idea/hunch about where the invalid memory access actually occurs?

https://github.com/chromium/chromium/blob/ba9748e78ec7e9c0d5...

duneroadrunner

قبل 7 سنواتdiscuss

Ug. After closer inspection, it looks like those particular raw pointers seem to be managed by a garbage collector. (Specifically, the "Blink GC" [1].) As others have pointed out, this particular bug may not actually be a C++ issue. (Or at least not a typical one.)

[1] https://chromium.googlesource.com/chromium/src/+/master/thir...

duneroadrunner

قبل 7 سنواتdiscuss

They are working on it. The analogue to the borrow checker in C++ is called the "lifetime profile checker" and (an incomplete version) is included in MS Visual C++, but last time I checked (in January) it seemed to still have too many false positives to be practical.

In the mean time, I think "the minimum necessary changes" to achieve memory and data race safety is to replace all your unsafe C++ elements (pointers, arrays, vectors, string_views, etc.) with compatible substitutes from the SaferCPlusPlus library [1]. You don't even need to replace them all at once. You can replace them incrementally and your code will continue to compile and run throughout the process. And where needed, maintain maximal performance as well [2].

[1] shameless plug: https://github.com/duneroadrunner/SaferCPlusPlus

[2] https://github.com/duneroadrunner/SaferCPlusPlus-BenchmarksG...

duneroadrunner

قبل 7 سنواتdiscuss

I suggest that the most expedient (cheapest) language to migrate the existing code base to would be a memory safe subset of C++ [1]. In practice most of the safety benefit could be obtained from a just a partial migration. Specifically, just banning raw pointers/views/spans and non-bounds-checked arrays and vectors. From a quick glance at the code in the (quite small) patch diff, the code in question includes:

    DOMArrayBuffer* result = DOMArrayBuffer::Create(raw_data_->ToArrayBuffer());
    
    ...
    
        raw_data_.reset();
    
    ...
    
   return result;

I'd imagine the effort/time/money it would take to replace those raw pointers with memory safe substitutes [2][3][4] (and enforce the ban going forward) would be relatively modest. Performance shouldn't be an issue [5].

[1] shameless plug: https://github.com/duneroadrunner/SaferCPlusPlus

[2] https://github.com/duneroadrunner/SaferCPlusPlus#registered-...

[3] https://github.com/duneroadrunner/SaferCPlusPlus#norad-point...

[4] https://github.com/duneroadrunner/SaferCPlusPlus#scope-point...

[5] https://github.com/duneroadrunner/SaferCPlusPlus-BenchmarksG...

comments