This is an archive of the discontinued LLVM Phabricator instance.

Differential D98006

Fixed Test for cases when DISABLE_NEW_COUNT is defined
AbandonedPublic

Authored by ldionne on Mar 4 2021, 10:25 PM.

Details

Reviewers
kamleshbhalui
Group Reviewers
Restricted Project
Summary

When DISABLE_NEW_COUNT is defined,
The test should not use globalMemCounter,
because corresponding operator new and operator delete
which does the counting will not be present.

Diff Detail

Event Timeline

kamleshbhalui requested review of this revision.Mar 4 2021, 10:25 PM
kamleshbhalui created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2021, 10:25 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
kamleshbhalui added a reviewer: ldionne.Mar 4 2021, 10:26 PM
Harbormaster completed remote builds in B92225: Diff 328390.Mar 4 2021, 11:43 PM
kamleshbhalui updated this revision to Diff 328428.Mar 5 2021, 1:06 AM

Fixed more tests

kamleshbhalui updated this revision to Diff 328438.Mar 5 2021, 1:29 AM

Added one fixed test

Harbormaster completed remote builds in B92250: Diff 328428.Mar 5 2021, 3:42 AM
Harbormaster completed remote builds in B92257: Diff 328438.Mar 5 2021, 5:15 AM
curdeius added a subscriber: curdeius.Mar 5 2021, 5:40 AM

Can you give some details please about the config on which these tests failed?

ldionne added a comment.Mar 5 2021, 6:11 AM
In D98006#2606418, @curdeius wrote:

Can you give some details please about the config on which these tests failed?

Yeah, and generally speaking, it's good to have a slightly more descriptive commit message than "Fix tests". Can you please update that too?

kamleshbhalui retitled this revision from Fixed test to Fixed Test for cases when DISABLE_NEW_COUNT is defined.Mar 5 2021, 6:58 AM
kamleshbhalui edited the summary of this revision. (Show Details)
kamleshbhalui added a comment.Mar 5 2021, 7:01 AM
In D98006#2606418, @curdeius wrote:

Can you give some details please about the config on which these tests failed?

just defined DISABLE_NEW_COUNT in <count_new.h>.

curdeius added a comment.Mar 5 2021, 7:06 AM

This doesn't answer my question unfortunately.
What configuration (platform/OS/compiler, cmake invocation etc.) you used when you found the failures?
What were these failures precisely?

kamleshbhalui added a comment.Mar 5 2021, 7:20 AM
In D98006#2606717, @curdeius wrote:

This doesn't answer my question unfortunately.
What configuration (platform/OS/compiler, cmake invocation etc.) you used when you found the failures?
What were these failures precisely?

I have defined DISABLE_NEW_COUNT macro manually in https://github.com/llvm/llvm-project/blob/main/libcxx/test/support/count_new.h

Here is the compiler verbose for one of the test.
clang version 11.0.0 (https://github.com/llvm/llvm-project.git 0160ad802e899c2922bc9b29564080c22eb0908c)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/8
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Candidate multilib: x32;@mx32
Selected multilib: .;@m64
"/usr/local/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/bin/clang-11" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name insert_iter_size_value.pass.cpp -mrelocation-model static -mframe-pointer=all -fmath-errno -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -fno-split-dwarf-inlining -debugger-tuning=gdb -v -nostdinc++ -resource-dir /usr/local/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/lib/clang/11.0.0 -include /home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/projects/libcxx/__config_site -include /home/kk/kkumar/tcllvm/llvm-project/libcxx/test/support/nasty_macros.h -I /home/kk/kkumar/tcllvm/llvm-project/libcxx/include -I /home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/projects/libcxx/include/c++build -D STDC_FORMAT_MACROS -D STDC_LIMIT_MACROS -D STDC_CONSTANT_MACROS -I /home/kk/kkumar/tcllvm/llvm-project/libcxx/test/support -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D _LIBCPP_DISABLE_AVAILABILITY -D _LIBCPP_HAS_THREAD_API_PTHREAD -D _LIBCPP_ABI_VERSION=1 -internal-isystem /usr/local/include -internal-isystem /usr/local/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/lib/clang/11.0.0/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -Werror -Wall -Wextra -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Werror=thread-safety -Wuser-defined-warnings -Wno-macro-redefined -Wno-macro-redefined -std=c++2a -fdeprecated-macro -fdebug-compilation-dir /home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/projects/libcxx/test/std/containers/sequences/list/list.modifiers -ferror-limit 19 -fcoroutines-ts -fgnuc-version=4.2.1 -fno-implicit-modules -fcxx-exceptions -fexceptions -faddrsig -o /tmp/insert_iter_size_value-8ddcc3.o -x c++ /home/kk/kkumar/tcllvm/llvm-project/libcxx/test/std/containers/sequences/list/list.modifiers/insert_iter_size_value.pass.cpp
clang -cc1 version 11.0.0 based upon LLVM 11.0.0 default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/home/kk/kkumar/tcllvm/llvm-project/libcxx/include
/home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/projects/libcxx/include/c++build
/home/kk/kkumar/tcllvm/llvm-project/libcxx/test/support
/usr/local/include
/usr/local/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/lib/clang/11.0.0/include
/usr/include/x86_64-linux-gnu
/usr/include
End of search list.
"/usr/bin/ld" -z relro --hash-style=gnu --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o /home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/projects/libcxx/test/std/containers/sequences/list/list.modifiers/Output/insert_iter_size_value.pass.cpp.dir/t.tmp.exe /usr/lib/gcc/x86_64-linux-gnu/9/../../../x86_64-linux-gnu/crt1.o /usr/lib/gcc/x86_64-linux-gnu/9/../../../x86_64-linux-gnu/crti.o /usr/lib/gcc/x86_64-linux-gnu/9/crtbegin.o -L/home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/./lib -L/usr/lib/gcc/x86_64-linux-gnu/9 -L/usr/lib/gcc/x86_64-linux-gnu/9/../../../x86_64-linux-gnu -L/usr/lib/gcc/x86_64-linux-gnu/9/../../../../lib64 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib64 -L/usr/lib/x86_64-linux-gnu/../../lib64 -L/usr/lib/gcc/x86_64-linux-gnu/9/../../.. -L/usr/local/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/bin/../lib -L/lib -L/usr/lib /tmp/insert_iter_size_value-8ddcc3.o -rpath /home/kk/kkumar/tcllvm/llvm-project/build-libcxx-relese/./lib -lc++ -lm -lgcc_s -lgcc -lpthread -lc -lgcc_s -lgcc -latomic -lc++experimental /usr/lib/gcc/x86_64-linux-gnu/9/crtend.o /usr/lib/gcc/x86_64-linux-gnu/9/../../../x86_64-linux-gnu/crtn.o
t.tmp.exe: /home/kk/kkumar/tcllvm/llvm-project/libcxx/test/std/containers/sequences/list/list.modifiers/insert_iter_size_value.pass.cpp:37: void test() [List = std::1::list<int, std::__1::allocator<int>>]: Assertion `false' failed.

Failed Tests (1):

libc++ :: std/containers/sequences/list/list.modifiers/insert_iter_size_value.pass.cpp

Testing Time: 0.82s

Failed: 1
• Quuxplusone added a subscriber: • Quuxplusone.Mar 5 2021, 9:53 AM

I don't know if "defining that macro manually" is expected to be a supported configuration.
That said, source-diving suggests to me that the macro is automatically defined whenever the sanitizers are turned on.
Does anyone test libc++ with sanitizers? (Does buildkite?) Could someone perhaps confirm that this fixes some specific sanitizer-related test failure?

DISABLE_NEW_COUNT is already checked in two places in the test suite:
std/containers/sequences/list/list.modifiers/insert_iter_value.pass.cpp
std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/auto_ptr.pass.cpp
but I have not looked into the details or history.

ldionne requested changes to this revision.Oct 14 2021, 8:53 AM

I *think* the intent of this patch is to be able to run these tests when the sanitizers are enabled. That would explain the difference between the tests touched by this review and those found by @Quuxplusone.

IMO we can make this change, but let's start running those tests on the sanitizers.

Also, to answer @Quuxplusone 's question, yes we do test the sanitizers on BuildKite (look for generic-asan & friends in run-buildbot).

libcxx/test/std/containers/sequences/list/list.modifiers/insert_iter_size_value.pass.cpp
13
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/nullptr_t_deleter_throw.pass.cpp
10 ↗(On Diff #328438)
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer_deleter_throw.pass.cpp
10 ↗(On Diff #328438)
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/unique_ptr.pass.cpp
9 ↗(On Diff #328438)
This revision now requires changes to proceed.Oct 14 2021, 8:53 AM
ldionne commandeered this revision.Sep 5 2023, 2:20 PM
ldionne edited reviewers, added: kamleshbhalui; removed: ldionne.

[Github PR transition cleanup]

This was done already and we run the tests using the sanitizers now, so I will abandon this.

Herald added a project: Restricted Project. · View Herald TranscriptSep 5 2023, 2:20 PM
ldionne abandoned this revision.Sep 5 2023, 2:20 PM

Revision Contents

PathSize
libcxx/
test/
std/
containers/
sequences/
list/
list.modifiers/
2 lines

Diff 328390

libcxx/test/std/containers/sequences/list/list.modifiers/insert_iter_size_value.pass.cpp

//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// <list> // <list>
// iterator insert(const_iterator position, size_type n, const value_type& x); // iterator insert(const_iterator position, size_type n, const value_type& x);
// UNSUPPORTED: sanitizer-new-delete // UNSUPPORTED: sanitizer-new-delete
ldionneAuthorUnsubmitted
Not Done
ReplyInline Actions
// iterator insert(const_iterator position, size_type n, const value_type& x);
- // UNSUPPORTED: sanitizer-new-delete
+
#include <list>
ldionne:
#include <list> #include <list>
#include <cstdlib> #include <cstdlib>
#include <cassert> #include <cassert>
#include "min_allocator.h" #include "min_allocator.h"
#include "count_new.h" #include "count_new.h"
#include "test_macros.h" #include "test_macros.h"
template <class List> template <class List>
void test() { void test() {
int a1[] = {1, 2, 3}; int a1[] = {1, 2, 3};
int a2[] = {1, 4, 4, 4, 4, 4, 2, 3}; int a2[] = {1, 4, 4, 4, 4, 4, 2, 3};
List l1(a1, a1+3); List l1(a1, a1+3);
typename List::iterator i = l1.insert(next(l1.cbegin()), 5, 4); typename List::iterator i = l1.insert(next(l1.cbegin()), 5, 4);
assert(i == next(l1.begin())); assert(i == next(l1.begin()));
assert(l1 == List(a2, a2+8)); assert(l1 == List(a2, a2+8));
#ifndef TEST_HAS_NO_EXCEPTIONS #if !defined(TEST_HAS_NO_EXCEPTIONS) && !defined(DISABLE_NEW_COUNT)
globalMemCounter.throw_after = 4; globalMemCounter.throw_after = 4;
int save_count = globalMemCounter.outstanding_new; int save_count = globalMemCounter.outstanding_new;
try try
{ {
i = l1.insert(i, 5, 5); i = l1.insert(i, 5, 5);
assert(false); assert(false);
} }
catch (...) catch (...)
Show All 16 Lines