From 7fc66d316d707fb9473e8e9bb57495f7c0a3c52c Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Tue, 4 Aug 2020 11:06:30 +0300 Subject: [PATCH] Compile cpp code with -Wall (#4310) -Wunused-function is temporarily disabled because it conflicts with interesting but unused functions. When we will have C++ tests on runtime, these interesting functions will get tested and so stop being unused. --- .../org/jetbrains/kotlin/CompileToBitcode.kt | 2 ++ common/src/hash/cpp/Base64.cpp | 6 +++++- common/src/hash/cpp/City.cpp | 4 ---- common/src/hash/cpp/Names.cpp | 1 - runtime/src/main/cpp/KString.cpp | 4 +--- runtime/src/main/cpp/Memory.cpp | 15 +++++++++++---- runtime/src/main/cpp/ObjCExport.mm | 4 +--- runtime/src/main/cpp/Porting.cpp | 3 +-- runtime/src/main/cpp/Worker.cpp | 7 ------- 9 files changed, 21 insertions(+), 25 deletions(-) diff --git a/build-tools/src/main/kotlin/org/jetbrains/kotlin/CompileToBitcode.kt b/build-tools/src/main/kotlin/org/jetbrains/kotlin/CompileToBitcode.kt index 270be957c59..5b21e2d5a7a 100644 --- a/build-tools/src/main/kotlin/org/jetbrains/kotlin/CompileToBitcode.kt +++ b/build-tools/src/main/kotlin/org/jetbrains/kotlin/CompileToBitcode.kt @@ -67,6 +67,8 @@ open class CompileToBitcode @Inject constructor(@InputDirectory val srcRoot: Fil "-Werror", "-ftls-model=initial-exec", "-Wno-unused-function") Language.CPP -> listOfNotNull("-std=c++14", "-Werror", "-O2", + "-Wall", + "-Wno-unused-function", // TODO: Enable this warning when we have C++ runtime tests. "-fPIC".takeIf { !HostManager().targetByName(target).isMINGW }) } return commonFlags + languageFlags + compilerArgs diff --git a/common/src/hash/cpp/Base64.cpp b/common/src/hash/cpp/Base64.cpp index d03d96452fc..031b4dc071e 100644 --- a/common/src/hash/cpp/Base64.cpp +++ b/common/src/hash/cpp/Base64.cpp @@ -136,7 +136,11 @@ int DecodeBase64( size_t buf = 0, len = 0; while (in < end) { - unsigned char c = kDecode[*in++]; + // char may be a signed type. Explicitly convert it to an unsigned byte, + // so that indexing into kDecode array is safe. The latter has exactly + // 256 elements, so any unsigned byte value is valid. + uint8_t index = *in++; + unsigned char c = kDecode[index]; switch (c) { case WHITESPACE: continue; /* skip whitespace */ diff --git a/common/src/hash/cpp/City.cpp b/common/src/hash/cpp/City.cpp index 31d4eefef98..cb263dc926f 100644 --- a/common/src/hash/cpp/City.cpp +++ b/common/src/hash/cpp/City.cpp @@ -26,10 +26,6 @@ static const uint64_t k0 = 0xc3a5c85c97cb3127ULL; static const uint64_t k1 = 0xb492b66fbe98f273ULL; static const uint64_t k2 = 0x9ae16a3b2f90404fULL; -// Magic numbers for 32-bit hashing. Copied from Murmur3. -static const uint32_t c1 = 0xcc9e2d51; -static const uint32_t c2 = 0x1b873593; - uint64_t UNALIGNED_LOAD64(const char *p) { uint64_t result; memcpy(&result, p, sizeof(result)); diff --git a/common/src/hash/cpp/Names.cpp b/common/src/hash/cpp/Names.cpp index e4ffa15a40b..0016f40db82 100644 --- a/common/src/hash/cpp/Names.cpp +++ b/common/src/hash/cpp/Names.cpp @@ -29,7 +29,6 @@ constexpr uint32_t PrintableHexSize(uint32_t input_length) { void PrintableHex(const uint8_t* data, uint32_t data_length, char* hex) { static const char* hex_digits = "0123456789ABCDEF"; - int i = 0; for(int i = 0; i < data_length; ++i) { *hex++ = hex_digits[(*data >> 4) & 0xf]; *hex++ = hex_digits[(*data++) & 0xf]; diff --git a/runtime/src/main/cpp/KString.cpp b/runtime/src/main/cpp/KString.cpp index 0763cebb0d2..a0658880b9f 100644 --- a/runtime/src/main/cpp/KString.cpp +++ b/runtime/src/main/cpp/KString.cpp @@ -44,7 +44,7 @@ OBJ_GETTER(utf8ToUtf16Impl, const char* rawString, const char* end, uint32_t cha if (rawString == nullptr) RETURN_OBJ(nullptr); ArrayHeader* result = AllocArrayInstance(theStringTypeInfo, charCount, OBJ_RESULT)->array(); KChar* rawResult = CharArrayAddressOfElementAt(result, 0); - auto convertResult = conversion(rawString, end, rawResult); + conversion(rawString, end, rawResult); RETURN_OBJ(result->obj()); } @@ -1091,7 +1091,6 @@ KInt Kotlin_String_lastIndexOfChar(KString thiz, KChar ch, KInt fromIndex) { if (fromIndex >= thiz->count_) { fromIndex = thiz->count_ - 1; } - KInt count = thiz->count_; KInt index = fromIndex; const KChar* thizRaw = CharArrayAddressOfElementAt(thiz, index); while (index >= 0) { @@ -1116,7 +1115,6 @@ KInt Kotlin_String_indexOfString(KString thiz, KString other, KInt fromIndex) { if (other->count_ == 0) { return fromIndex; } - KInt count = thiz->count_; const KChar* thizRaw = CharArrayAddressOfElementAt(thiz, fromIndex); const KChar* otherRaw = CharArrayAddressOfElementAt(other, 0); void* result = konan::memmem(thizRaw, (thiz->count_ - fromIndex) * sizeof(KChar), diff --git a/runtime/src/main/cpp/Memory.cpp b/runtime/src/main/cpp/Memory.cpp index 5f9945b4659..36badfdc08f 100644 --- a/runtime/src/main/cpp/Memory.cpp +++ b/runtime/src/main/cpp/Memory.cpp @@ -119,13 +119,18 @@ typedef KStdUnorderedMap> KThreadLocalStorageMap; // Prevents clang from replacing FrameOverlay struct // with single pointer. // Can be removed when FrameOverlay will become more complex. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-variable" FrameOverlay exportFrameOverlay; +#pragma clang diagnostic pop // Current number of allocated containers. volatile int allocCount = 0; volatile int aliveMemoryStatesCount = 0; +#if USE_CYCLIC_GC KBoolean g_hasCyclicCollector = true; +#endif // USE_CYCLIC_GC // TODO: can we pass this variable as an explicit argument? THREAD_LOCAL_VARIABLE MemoryState* memoryState = nullptr; @@ -980,7 +985,7 @@ void freeContainer(ContainerHeader* container) { runDeallocationHooks(container); // Now let's clean all object's fields in this container. - traverseContainerObjectFields(container, [container](ObjHeader** location) { + traverseContainerObjectFields(container, [](ObjHeader** location) { ZeroHeapRef(location); }); @@ -1017,7 +1022,7 @@ void depthFirstTraversal(ContainerHeader* start, bool* hasCycles, continue; } toVisit.push_front(markAsRemoved(container)); - traverseContainerReferredObjects(container, [container, hasCycles, firstBlocker, &order, &toVisit](ObjHeader* obj) { + traverseContainerReferredObjects(container, [container, hasCycles, firstBlocker, &toVisit](ObjHeader* obj) { if (*firstBlocker != nullptr) return; if (obj->has_meta_object() && ((obj->meta_object()->flags_ & MF_NEVER_FROZEN) != 0)) { @@ -1426,7 +1431,7 @@ void collectWhite(MemoryState* state, ContainerHeader* start) { toVisit.pop_front(); if (container->color() != CONTAINER_TAG_GC_WHITE || container->buffered()) continue; container->setColorAssertIfGreen(CONTAINER_TAG_GC_BLACK); - traverseContainerObjectFields(container, [state, &toVisit](ObjHeader** location) { + traverseContainerObjectFields(container, [&toVisit](ObjHeader** location) { auto* ref = *location; if (ref == nullptr) return; auto* childContainer = ref->container(); @@ -1610,7 +1615,9 @@ void decrementStack(MemoryState* state) { void garbageCollect(MemoryState* state, bool force) { RuntimeAssert(!state->gcInProgress, "Recursive GC is disallowed"); +#if TRACE_GC uint64_t allocSinceLastGc = state->allocSinceLastGc; +#endif // TRACE_GC state->allocSinceLastGc = 0; if (!IsStrictMemoryModel) { @@ -2493,7 +2500,7 @@ void freezeAcyclic(ContainerHeader* rootContainer, ContainerHeaderSet* newlyFroz newlyFrozen->insert(current); MEMORY_LOG("freezing %p\n", current) current->freeze(); - traverseContainerReferredObjects(current, [current, &queue](ObjHeader* obj) { + traverseContainerReferredObjects(current, [&queue](ObjHeader* obj) { ContainerHeader* objContainer = obj->container(); if (canFreeze(objContainer)) { if (objContainer->marked()) diff --git a/runtime/src/main/cpp/ObjCExport.mm b/runtime/src/main/cpp/ObjCExport.mm index 6775ce2631a..fbd15db6b0a 100644 --- a/runtime/src/main/cpp/ObjCExport.mm +++ b/runtime/src/main/cpp/ObjCExport.mm @@ -799,8 +799,6 @@ static void throwIfCantBeOverridden(Class clazz, const KotlinToObjCMethodAdapter } static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, const TypeInfo* fieldsInfo) { - Class superClass = class_getSuperclass(clazz); - KStdUnorderedSet definedSelectors; addDefinedSelectors(clazz, definedSelectors); @@ -913,7 +911,7 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, co auto interfaceVTablesIt = interfaceVTables.find(interfaceId); if (interfaceVTablesIt == interfaceVTables.end()) { itableEqualsSuper = false; - interfaceVTables.emplace(interfaceId, std::move(KStdVector(interfaceVTableSize))); + interfaceVTables.emplace(interfaceId, KStdVector(interfaceVTableSize)); } else { auto const& interfaceVTable = interfaceVTablesIt->second; RuntimeAssert(interfaceVTable.size() == interfaceVTableSize, ""); diff --git a/runtime/src/main/cpp/Porting.cpp b/runtime/src/main/cpp/Porting.cpp index 58a1d59e48a..eae63ad353c 100644 --- a/runtime/src/main/cpp/Porting.cpp +++ b/runtime/src/main/cpp/Porting.cpp @@ -85,7 +85,7 @@ int getLastErrorMessage(char* message, uint32_t size) { auto errMsgBufSize = size / 4; wchar_t errMsgBuffer[errMsgBufSize]; ::FormatMessageW(flags, NULL, errCode, 0, errMsgBuffer, errMsgBufSize, NULL); - auto errMsgLength = ::WideCharToMultiByte(CP_UTF8, 0, errMsgBuffer, -1, message, size, NULL, NULL); + ::WideCharToMultiByte(CP_UTF8, 0, errMsgBuffer, -1, message, size, NULL, NULL); } return errCode; } @@ -108,7 +108,6 @@ int32_t consoleReadUtf8(void* utf8, uint32_t maxSizeBytes) { if (!length && KonanNeedDebugInfo) { char msg[512]; auto errCode = getLastErrorMessage(msg, sizeof(msg)); - char buffer[1024]; consoleErrorf("UTF-16 to UTF-8 conversion error %d: %s", errCode, msg); } ((char*) utf8)[length] = 0; diff --git a/runtime/src/main/cpp/Worker.cpp b/runtime/src/main/cpp/Worker.cpp index be75ae930e9..b9f58b9b199 100644 --- a/runtime/src/main/cpp/Worker.cpp +++ b/runtime/src/main/cpp/Worker.cpp @@ -165,11 +165,6 @@ class Worker { pthread_t thread_ = 0; }; -#else // WITH_WORKERS -class Worker { - KInt id; -}; - #endif // WITH_WORKERS namespace { @@ -407,7 +402,6 @@ class State { } OBJ_GETTER(getWorkerNameUnlocked, KInt id) { - Worker* worker = nullptr; ObjHolder nameHolder; { Locker locker(&lock_); @@ -566,7 +560,6 @@ KInt currentWorker() { } KInt execute(KInt id, KInt transferMode, KRef producer, KNativePtr jobFunction) { - Job job; ObjHolder holder; WorkerLaunchpad(producer, holder.slot()); KNativePtr jobArgument = transfer(&holder, transferMode);