Closed Bug 1594942 Opened 2 years ago Closed 2 years ago

null pointer passed as argument 2, which is declared to never be null in include/nsTArray.h:586


(Core :: XPCOM, defect)

Not set



Tracking Status
firefox72 --- wontfix
firefox73 --- wontfix
firefox74 --- fixed


(Reporter: tsmith, Assigned: Waldo)


(Blocks 2 open bugs)


(Keywords: csectype-undefined)


(1 file)

This is triggered when running gtests with an UBSan build. Looks like a null pointer is passed to strcpy.

To enable this check add the following to your mozconfig:

ac_add_options --enable-address-sanitizer
ac_add_options --enable-undefined-sanitizer="nonnull-attribute"
ac_add_options --disable-jemalloc
[ RUN      ] TArray.test_int_array
src/objdir-ff-ubsan/dist/include/nsTArray.h:586:32: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:28: note: nonnull attribute specified here
    #0 0x7fd143f9f802 in void AssignRangeAlgorithm<true, true>::implementation<int, int, unsigned long, unsigned long>(int*, unsigned long, unsigned long, int const*) src/objdir-ff-ubsan/dist/include/nsTArray.h:586:5
    #1 0x7fd143f9f802 in void nsTArray_Impl<int, nsTArrayInfallibleAllocator>::AssignRange<int>(unsigned long, unsigned long, int const*) src/objdir-ff-ubsan/dist/include/nsTArray.h:2260
    #2 0x7fd143f9f802 in int* nsTArray_Impl<int, nsTArrayInfallibleAllocator>::AppendElements<int, nsTArrayInfallibleAllocator>(int const*, unsigned long) src/objdir-ff-ubsan/dist/include/nsTArray.h:2427
    #3 0x7fd1440662a2 in bool TestTArray::test_basic_array<int>(int*, unsigned long, int const&) src/xpcom/tests/gtest/TestTArray2.cpp:191:7
    #4 0x7fd1440662a2 in TestTArray::TArray_test_int_array_Test::TestBody() src/xpcom/tests/gtest/TestTArray2.cpp:204
    #5 0x7fd1449711f5 in testing::Test::Run() src/testing/gtest/gtest/src/
    #6 0x7fd144972b53 in testing::TestInfo::Run() src/testing/gtest/gtest/src/
    #7 0x7fd144973c87 in testing::TestCase::Run() src/testing/gtest/gtest/src/
    #8 0x7fd14498986e in testing::internal::UnitTestImpl::RunAllTests() src/testing/gtest/gtest/src/
    #9 0x7fd144988e17 in testing::UnitTest::Run() src/testing/gtest/gtest/src/
    #10 0x7fd1449bbf00 in RUN_ALL_TESTS() src/objdir-ff-ubsan/dist/include/gtest/gtest.h:2342:46
    #11 0x7fd1449bbf00 in mozilla::RunGTestFunc(int*, char**) src/testing/gtest/mozilla/GTestRunner.cpp:158
    #12 0x7fd152f3e92d in XREMain::XRE_mainStartup(bool*) src/toolkit/xre/nsAppRunner.cpp:3774:16
    #13 0x7fd152f50ffc in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4708:12
    #14 0x7fd152f52893 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4802:21
    #15 0x558ccdbce99e in do_main(int, char**, char**) src/browser/app/nsBrowserApp.cpp:218:22
    #16 0x558ccdbce99e in main src/browser/app/nsBrowserApp.cpp:300

I'm not sure if there is a missing null check somewhere or if this is just a bug in the test.

It was last changed a very long time ago by :Waldo.

Flags: needinfo?(jwalden)

The function in question doesn't document its pointer parameter as being required to be non-null. Nor does the function it delegates behavior to, do so. So if the test is doing something wrong, it's not doing something that's been clearly forbidden.

I don't have opinions as to whether this function should have a must-not-be-null requirement imposed on it -- this test maybe suggests no? bug 317959 added this fifteen years ago wanting this not to crash, arguably establishing an implicit and undocumented requirement -- but if it shouldn't have such imposed, AssignRangeAlgorithm<true, true>::implementation needs to null-check aValues before performing the copy.

Flags: needinfo?(jwalden)
Assignee: nobody → jwalden
Pushed by
Null-check the source of a range assignment in POD nsTArray before memcpying from it (because memcpy requires its arguments be non-null, even for zero-count data).  r=froydnj
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
Blocks: 1640253
You need to log in before you can comment on or make changes to this bug.