From: Jeff Genovy <29107334+jefgen@users.noreply.github.com> Date: Thu, 30 Aug 2018 01:50:50 +0000 (-0700) Subject: ICU-13712 ICU4C does not report OOM if it fails to memory map the data file(s) (#30) X-Git-Tag: release-63-rc~93 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=eae698a61e2669eb94e2a1478c8e051a1fab982a;p=icu ICU-13712 ICU4C does not report OOM if it fails to memory map the data file(s) (#30) ICU does not report Out-Of-Memory (OOM) if it fails to memory map the data file(s) when calling the various platform API(s) to do so. When you are using ICU with memory-mapped data file(s), and ICU fails to map the data file due to being out-of-memory, it does not bubble this failure up to the API that was called. You will instead get back the error U_MISSING_RESOURCE_ERROR, rather than U_MEMORY_ALLOCATION_ERROR, which might be a bit surprising to the caller of the API. This can lead to the application thinking that there are no resources for "en_US" or "en" (or even "root"). This change modifies ICU4C so that it will report back U_MEMORY_ALLOCATION_ERROR if OOM happens when attempting to load the data files. --- diff --git a/icu4c/source/common/udata.cpp b/icu4c/source/common/udata.cpp index 1905804857f..efcd2a2f975 100644 --- a/icu4c/source/common/udata.cpp +++ b/icu4c/source/common/udata.cpp @@ -759,16 +759,19 @@ openCommonData(const char *path, /* Path from OpenChoice? */ UDataPathIterator iter(u_getDataDirectory(), inBasename, path, ".dat", TRUE, pErrorCode); - while((UDataMemory_isLoaded(&tData)==FALSE) && (pathBuffer = iter.next(pErrorCode)) != NULL) + while ((UDataMemory_isLoaded(&tData)==FALSE) && (pathBuffer = iter.next(pErrorCode)) != NULL) { #ifdef UDATA_DEBUG fprintf(stderr, "ocd: trying path %s - ", pathBuffer); #endif - uprv_mapFile(&tData, pathBuffer); + uprv_mapFile(&tData, pathBuffer, pErrorCode); #ifdef UDATA_DEBUG fprintf(stderr, "%s\n", UDataMemory_isLoaded(&tData)?"LOADED":"not loaded"); #endif } + if (U_FAILURE(*pErrorCode)) { + return NULL; + } #if defined(OS390_STUBDATA) && defined(OS390BATCH) if (!UDataMemory_isLoaded(&tData)) { @@ -777,7 +780,7 @@ openCommonData(const char *path, /* Path from OpenChoice? */ uprv_strncpy(ourPathBuffer, path, 1019); ourPathBuffer[1019]=0; uprv_strcat(ourPathBuffer, ".dat"); - uprv_mapFile(&tData, ourPathBuffer); + uprv_mapFile(&tData, ourPathBuffer, pErrorCode); } #endif @@ -868,7 +871,7 @@ static UBool extendICUData(UErrorCode *pErr) umtx_unlock(&extendICUDataMutex); #endif return didUpdate; /* Return true if ICUData pointer was updated. */ - /* (Could potentialy have been done by another thread racing */ + /* (Could potentially have been done by another thread racing */ /* us through here, but that's fine, we still return true */ /* so that current thread will also examine extended data. */ } @@ -994,12 +997,12 @@ static UDataMemory *doLoadFromIndividualFiles(const char *pkgName, /* init path iterator for individual files */ UDataPathIterator iter(dataPath, pkgName, path, tocEntryPathSuffix, FALSE, pErrorCode); - while((pathBuffer = iter.next(pErrorCode)) != NULL) + while ((pathBuffer = iter.next(pErrorCode)) != NULL) { #ifdef UDATA_DEBUG fprintf(stderr, "UDATA: trying individual file %s\n", pathBuffer); #endif - if(uprv_mapFile(&dataMemory, pathBuffer)) + if (uprv_mapFile(&dataMemory, pathBuffer, pErrorCode)) { pEntryData = checkDataItem(dataMemory.pHeader, isAcceptable, context, type, name, subErrorCode, pErrorCode); if (pEntryData != NULL) { @@ -1015,7 +1018,7 @@ static UDataMemory *doLoadFromIndividualFiles(const char *pkgName, return pEntryData; } - /* the data is not acceptable, or some error occured. Either way, unmap the memory */ + /* the data is not acceptable, or some error occurred. Either way, unmap the memory */ udata_close(&dataMemory); /* If we had a nasty error, bail out completely. */ @@ -1084,6 +1087,11 @@ static UDataMemory *doLoadFromCommonData(UBool isICUData, const char * /*pkgName } } } + // If we failed due to being out-of-memory, then stop early and report the error. + if (*subErrorCode == U_MEMORY_ALLOCATION_ERROR) { + *pErrorCode = *subErrorCode; + return NULL; + } /* Data wasn't found. If we were looking for an ICUData item and there is * more data available, load it and try again, * otherwise break out of this loop. */ diff --git a/icu4c/source/common/umapfile.cpp b/icu4c/source/common/umapfile.cpp index 53699e762b2..ffb18ef152e 100644 --- a/icu4c/source/common/umapfile.cpp +++ b/icu4c/source/common/umapfile.cpp @@ -64,7 +64,7 @@ # include "unicode/udata.h" # define LIB_PREFIX "lib" # define LIB_SUFFIX ".dll" - /* This is inconvienient until we figure out what to do with U_ICUDATA_NAME in utypes.h */ + /* This is inconvenient until we figure out what to do with U_ICUDATA_NAME in utypes.h */ # define U_ICUDATA_ENTRY_NAME "icudt" U_ICU_VERSION_SHORT U_LIB_SUFFIX_C_NAME_STRING "_dat" # endif #elif MAP_IMPLEMENTATION==MAP_STDIO @@ -84,7 +84,10 @@ *----------------------------------------------------------------------------*/ #if MAP_IMPLEMENTATION==MAP_NONE U_CFUNC UBool - uprv_mapFile(UDataMemory *pData, const char *path) { + uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) { + if (U_FAILURE(*status)) { + return FALSE; + } UDataMemory_init(pData); /* Clear the output struct. */ return FALSE; /* no file access */ } @@ -97,12 +100,17 @@ uprv_mapFile( UDataMemory *pData, /* Fill in with info on the result doing the mapping. */ /* Output only; any original contents are cleared. */ - const char *path /* File path to be opened/mapped */ + const char *path, /* File path to be opened/mapped. */ + UErrorCode *status /* Error status, used to report out-of-memory errors. */ ) { HANDLE map; HANDLE file; - + + if (U_FAILURE(*status)) { + return FALSE; + } + UDataMemory_init(pData); /* Clear the output struct. */ /* open the input file */ @@ -132,7 +140,12 @@ // TODO: Is it worth setting extended parameters to specify random access? file = CreateFile2(utf16Path, GENERIC_READ, FILE_SHARE_READ, OPEN_EXISTING, NULL); #endif - if(file==INVALID_HANDLE_VALUE) { + if (file == INVALID_HANDLE_VALUE) { + // If we failed to open the file due to an out-of-memory error, then we want + // to report that error back to the caller. + if (HRESULT_FROM_WIN32(GetLastError()) == E_OUTOFMEMORY) { + *status = U_MEMORY_ALLOCATION_ERROR; + } return FALSE; } @@ -165,7 +178,12 @@ map = CreateFileMappingFromApp(file, NULL, PAGE_READONLY, 0, NULL); #endif CloseHandle(file); - if(map==NULL) { + if (map == NULL) { + // If we failed to create the mapping due to an out-of-memory error, then + // we want to report that error back to the caller. + if (HRESULT_FROM_WIN32(GetLastError()) == E_OUTOFMEMORY) { + *status = U_MEMORY_ALLOCATION_ERROR; + } return FALSE; } @@ -193,12 +211,16 @@ #elif MAP_IMPLEMENTATION==MAP_POSIX U_CFUNC UBool - uprv_mapFile(UDataMemory *pData, const char *path) { + uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) { int fd; int length; struct stat mystat; void *data; + if (U_FAILURE(*status)) { + return FALSE; + } + UDataMemory_init(pData); /* Clear the output struct. */ /* determine the length of the file */ @@ -221,6 +243,7 @@ #endif close(fd); /* no longer needed */ if(data==MAP_FAILED) { + // Possibly check the errno value for ENOMEM, and report U_MEMORY_ALLOCATION_ERROR? return FALSE; } @@ -263,11 +286,15 @@ } U_CFUNC UBool - uprv_mapFile(UDataMemory *pData, const char *path) { + uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) { FILE *file; int32_t fileLength; void *p; + if (U_FAILURE(*status)) { + return FALSE; + } + UDataMemory_init(pData); /* Clear the output struct. */ /* open the input file */ file=fopen(path, "rb"); @@ -286,6 +313,7 @@ p=uprv_malloc(fileLength); if(p==NULL) { fclose(file); + *status = U_MEMORY_ALLOCATION_ERROR; return FALSE; } @@ -351,7 +379,7 @@ * * TODO: This works the way ICU historically has, but the * whole data fallback search path is so complicated that - * proabably almost no one will ever really understand it, + * probably almost no one will ever really understand it, * the potential for confusion is large. (It's not just * this one function, but the whole scheme.) * @@ -391,7 +419,7 @@ # define DATA_TYPE "dat" - U_CFUNC UBool uprv_mapFile(UDataMemory *pData, const char *path) { + U_CFUNC UBool uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) { const char *inBasename; char *basename; char pathBuffer[1024]; @@ -399,6 +427,10 @@ dllhandle *handle; void *val=0; + if (U_FAILURE(*status)) { + return FALSE; + } + inBasename=uprv_strrchr(path, U_FILE_SEP_CHAR); if(inBasename==NULL) { inBasename = path; @@ -430,6 +462,7 @@ data=mmap(0, length, PROT_READ, MAP_PRIVATE, fd, 0); close(fd); /* no longer needed */ if(data==MAP_FAILED) { + // Possibly check the errorno value for ENOMEM, and report U_MEMORY_ALLOCATION_ERROR? return FALSE; } pData->map = (char *)data + length; diff --git a/icu4c/source/common/umapfile.h b/icu4c/source/common/umapfile.h index 24e476b11e9..92bd567a2a9 100644 --- a/icu4c/source/common/umapfile.h +++ b/icu4c/source/common/umapfile.h @@ -29,7 +29,7 @@ #include "unicode/udata.h" #include "putilimp.h" -U_CFUNC UBool uprv_mapFile(UDataMemory *pdm, const char *path); +U_CFUNC UBool uprv_mapFile(UDataMemory *pdm, const char *path, UErrorCode *status); U_CFUNC void uprv_unmapFile(UDataMemory *pData); /* MAP_NONE: no memory mapping, no file access at all */ diff --git a/icu4c/source/common/uresbund.cpp b/icu4c/source/common/uresbund.cpp index 2a8ec7292b4..656eeb7b442 100644 --- a/icu4c/source/common/uresbund.cpp +++ b/icu4c/source/common/uresbund.cpp @@ -367,7 +367,12 @@ static UResourceDataEntry *init_entry(const char *localeID, const char *path, UE /* this is the actual loading */ res_load(&(r->fData), r->fPath, r->fName, status); - if (U_FAILURE(*status)) { + if (U_FAILURE(*status)) { + /* if we failed to load due to an out-of-memory error, exit early. */ + if (*status == U_MEMORY_ALLOCATION_ERROR) { + uprv_free(r); + return NULL; + } /* we have no such entry in dll, so it will always use fallback */ *status = U_USING_FALLBACK_WARNING; r->fBogus = U_USING_FALLBACK_WARNING; @@ -537,6 +542,11 @@ loadParentsExceptRoot(UResourceDataEntry *&t1, UErrorCode usrStatus = U_ZERO_ERROR; if (usingUSRData) { // This code inserts user override data into the inheritance chain. u2 = init_entry(name, usrDataPath, &usrStatus); + // If we failed due to out-of-memory, report that to the caller and exit early. + if (usrStatus == U_MEMORY_ALLOCATION_ERROR) { + *status = usrStatus; + return FALSE; + } } if (usingUSRData && U_SUCCESS(usrStatus) && u2->fBogus == U_ZERO_ERROR) { @@ -642,21 +652,32 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID, /* We're going to skip all the locales that do not have any data */ r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus); + // If we failed due to out-of-memory, report the failure and exit early. + if (intStatus == U_MEMORY_ALLOCATION_ERROR) { + *status = intStatus; + goto finishUnlock; + } + if(r != NULL) { /* if there is one real locale, we can look for parents. */ t1 = r; hasRealData = TRUE; if ( usingUSRData ) { /* This code inserts user override data into the inheritance chain */ UErrorCode usrStatus = U_ZERO_ERROR; UResourceDataEntry *u1 = init_entry(t1->fName, usrDataPath, &usrStatus); - if ( u1 != NULL ) { - if(u1->fBogus == U_ZERO_ERROR) { - u1->fParent = t1; - r = u1; - } else { - /* the USR override data wasn't found, set it to be deleted */ - u1->fCountExisting = 0; - } - } + // If we failed due to out-of-memory, report the failure and exit early. + if (intStatus == U_MEMORY_ALLOCATION_ERROR) { + *status = intStatus; + goto finishUnlock; + } + if ( u1 != NULL ) { + if(u1->fBogus == U_ZERO_ERROR) { + u1->fParent = t1; + r = u1; + } else { + /* the USR override data wasn't found, set it to be deleted */ + u1->fCountExisting = 0; + } + } } if (hasChopped && !isRoot) { if (!loadParentsExceptRoot(t1, name, UPRV_LENGTHOF(name), usingUSRData, usrDataPath, status)) { @@ -671,6 +692,11 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID, /* insert default locale */ uprv_strcpy(name, uloc_getDefault()); r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus); + // If we failed due to out-of-memory, report the failure and exit early. + if (intStatus == U_MEMORY_ALLOCATION_ERROR) { + *status = intStatus; + goto finishUnlock; + } intStatus = U_USING_DEFAULT_WARNING; if(r != NULL) { /* the default locale exists */ t1 = r; @@ -690,6 +716,11 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID, if(r == NULL) { uprv_strcpy(name, kRootLocaleName); r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus); + // If we failed due to out-of-memory, report the failure and exit early. + if (intStatus == U_MEMORY_ALLOCATION_ERROR) { + *status = intStatus; + goto finishUnlock; + } if(r != NULL) { t1 = r; intStatus = U_USING_DEFAULT_WARNING;