bpo-30947: Update libexpat from 2.2.1 to 2.2.3 (#3106)

* bpo-30947: Update libexpat from 2.2.1 to 2.2.3

* Add NEWS entry

* Add new loadlibrary.c

* expat_external.h: restore include "pyexpatns.h"

* PCbuild: add expat/loadlibrary.c

* Define XML_POOR_ENTROPY to compile expat
diff --git a/Modules/expat/xmlparse.c b/Modules/expat/xmlparse.c
index daec151..b703e61 100644
--- a/Modules/expat/xmlparse.c
+++ b/Modules/expat/xmlparse.c
@@ -1,10 +1,12 @@
 /* Copyright (c) 1998, 1999, 2000 Thai Open Source Software Center Ltd
    See the file COPYING for copying permission.
 
-   77fea421d361dca90041d0040ecf1dca651167fadf2af79e990e35168d70d933 (2.2.1+)
+   101bfd65d1ff3d1511cf6671e6aae65f82cd97df6f4da137d46d510731830ad9 (2.2.3+)
 */
 
-#define _GNU_SOURCE 1                   /* syscall prototype */
+#if !defined(_GNU_SOURCE)
+# define _GNU_SOURCE 1                  /* syscall prototype */
+#endif
 
 #include <stddef.h>
 #include <string.h>                     /* memset(), memcpy() */
@@ -19,6 +21,8 @@
 #include <sys/time.h>                   /* gettimeofday() */
 #include <sys/types.h>                  /* getpid() */
 #include <unistd.h>                     /* getpid() */
+#include <fcntl.h>                      /* O_RDONLY */
+#include <errno.h>
 #endif
 
 #define XML_BUILDING_EXPAT 1
@@ -33,6 +37,57 @@
 #include "expat.h"
 #include "siphash.h"
 
+#if defined(HAVE_GETRANDOM) || defined(HAVE_SYSCALL_GETRANDOM)
+# if defined(HAVE_GETRANDOM)
+#  include <sys/random.h>    /* getrandom */
+# else
+#  include <unistd.h>        /* syscall */
+#  include <sys/syscall.h>   /* SYS_getrandom */
+# endif
+# if ! defined(GRND_NONBLOCK)
+#  define GRND_NONBLOCK  0x0001
+# endif  /* defined(GRND_NONBLOCK) */
+#endif  /* defined(HAVE_GETRANDOM) || defined(HAVE_SYSCALL_GETRANDOM) */
+
+#if defined(HAVE_LIBBSD) \
+    && (defined(HAVE_ARC4RANDOM_BUF) || defined(HAVE_ARC4RANDOM))
+# include <bsd/stdlib.h>
+#endif
+
+#if defined(_WIN32) && !defined(LOAD_LIBRARY_SEARCH_SYSTEM32)
+# define LOAD_LIBRARY_SEARCH_SYSTEM32  0x00000800
+#endif
+
+#if !defined(HAVE_GETRANDOM) && !defined(HAVE_SYSCALL_GETRANDOM) \
+    && !defined(HAVE_ARC4RANDOM_BUF) && !defined(HAVE_ARC4RANDOM) \
+    && !defined(XML_DEV_URANDOM) \
+    && !defined(_WIN32) \
+    && !defined(XML_POOR_ENTROPY)
+# error  \
+    You do not have support for any sources of high quality entropy \
+    enabled.  For end user security, that is probably not what you want. \
+    \
+    Your options include: \
+      * Linux + glibc >=2.25 (getrandom): HAVE_GETRANDOM, \
+      * Linux + glibc <2.25 (syscall SYS_getrandom): HAVE_SYSCALL_GETRANDOM, \
+      * BSD / macOS >=10.7 (arc4random_buf): HAVE_ARC4RANDOM_BUF, \
+      * BSD / macOS <10.7 (arc4random): HAVE_ARC4RANDOM, \
+      * libbsd (arc4random_buf): HAVE_ARC4RANDOM_BUF + HAVE_LIBBSD, \
+      * libbsd (arc4random): HAVE_ARC4RANDOM + HAVE_LIBBSD, \
+      * Linux / BSD / macOS (/dev/urandom): XML_DEV_URANDOM \
+      * Windows (RtlGenRandom): _WIN32. \
+    \
+    If insist on not using any of these, bypass this error by defining \
+    XML_POOR_ENTROPY; you have been warned. \
+    \
+    For CMake, one way to pass the define is: \
+        cmake -DCMAKE_C_FLAGS="-pipe -O2 -DHAVE_SYSCALL_GETRANDOM" . \
+    \
+    If you have reasons to patch this detection code away or need changes \
+    to the build system, please open a bug.  Thank you!
+#endif
+
+
 #ifdef XML_UNICODE
 #define XML_ENCODE_MAX XML_UTF16_ENCODE_MAX
 #define XmlConvert XmlUtf16Convert
@@ -436,6 +491,9 @@
 getElementType(XML_Parser parser, const ENCODING *enc,
                const char *ptr, const char *end);
 
+static XML_Char *copyString(const XML_Char *s,
+                            const XML_Memory_Handling_Suite *memsuite);
+
 static unsigned long generate_hash_secret_salt(XML_Parser parser);
 static XML_Bool startParsing(XML_Parser parser);
 
@@ -696,21 +754,13 @@
 
 
 #if defined(HAVE_GETRANDOM) || defined(HAVE_SYSCALL_GETRANDOM)
-# include <errno.h>
-
-# if defined(HAVE_GETRANDOM)
-#  include <sys/random.h>    /* getrandom */
-# else
-#  include <unistd.h>        /* syscall */
-#  include <sys/syscall.h>   /* SYS_getrandom */
-# endif
 
 /* Obtain entropy on Linux 3.17+ */
 static int
-writeRandomBytes_getrandom(void * target, size_t count) {
+writeRandomBytes_getrandom_nonblock(void * target, size_t count) {
   int success = 0;  /* full count bytes written? */
   size_t bytesWrittenTotal = 0;
-  const unsigned int getrandomFlags = 0;
+  const unsigned int getrandomFlags = GRND_NONBLOCK;
 
   do {
     void * const currentTarget = (void*)((char*)target + bytesWrittenTotal);
@@ -728,7 +778,7 @@
       if (bytesWrittenTotal >= count)
         success = 1;
     }
-  } while (! success && (errno == EINTR || errno == EAGAIN));
+  } while (! success && (errno == EINTR));
 
   return success;
 }
@@ -736,12 +786,67 @@
 #endif  /* defined(HAVE_GETRANDOM) || defined(HAVE_SYSCALL_GETRANDOM) */
 
 
+#if ! defined(_WIN32) && defined(XML_DEV_URANDOM)
+
+/* Extract entropy from /dev/urandom */
+static int
+writeRandomBytes_dev_urandom(void * target, size_t count) {
+  int success = 0;  /* full count bytes written? */
+  size_t bytesWrittenTotal = 0;
+
+  const int fd = open("/dev/urandom", O_RDONLY);
+  if (fd < 0) {
+    return 0;
+  }
+
+  do {
+    void * const currentTarget = (void*)((char*)target + bytesWrittenTotal);
+    const size_t bytesToWrite = count - bytesWrittenTotal;
+
+    const ssize_t bytesWrittenMore = read(fd, currentTarget, bytesToWrite);
+
+    if (bytesWrittenMore > 0) {
+      bytesWrittenTotal += bytesWrittenMore;
+      if (bytesWrittenTotal >= count)
+        success = 1;
+    }
+  } while (! success && (errno == EINTR));
+
+  close(fd);
+  return success;
+}
+
+#endif  /* ! defined(_WIN32) && defined(XML_DEV_URANDOM) */
+
+
+#if defined(HAVE_ARC4RANDOM)
+
+static void
+writeRandomBytes_arc4random(void * target, size_t count) {
+  size_t bytesWrittenTotal = 0;
+
+  while (bytesWrittenTotal < count) {
+    const uint32_t random32 = arc4random();
+    size_t i = 0;
+
+    for (; (i < sizeof(random32)) && (bytesWrittenTotal < count);
+        i++, bytesWrittenTotal++) {
+      const uint8_t random8 = (uint8_t)(random32 >> (i * 8));
+      ((uint8_t *)target)[bytesWrittenTotal] = random8;
+    }
+  }
+}
+
+#endif  /* defined(HAVE_ARC4RANDOM) */
+
+
 #ifdef _WIN32
 
 typedef BOOLEAN (APIENTRY *RTLGENRANDOM_FUNC)(PVOID, ULONG);
+HMODULE _Expat_LoadLibrary(LPCTSTR filename);  /* see loadlibrary.c */
 
 /* Obtain entropy on Windows XP / Windows Server 2003 and later.
- * Hint on RtlGenRandom and the following article from libsodioum.
+ * Hint on RtlGenRandom and the following article from libsodium.
  *
  * Michael Howard: Cryptographically Secure Random number on Windows without using CryptoAPI
  * https://blogs.msdn.microsoft.com/michael_howard/2005/01/14/cryptographically-secure-random-number-on-windows-without-using-cryptoapi/
@@ -749,7 +854,7 @@
 static int
 writeRandomBytes_RtlGenRandom(void * target, size_t count) {
   int success = 0;  /* full count bytes written? */
-  const HMODULE advapi32 = LoadLibrary("ADVAPI32.DLL");
+  const HMODULE advapi32 = _Expat_LoadLibrary(TEXT("ADVAPI32.DLL"));
 
   if (advapi32) {
     const RTLGENRANDOM_FUNC RtlGenRandom
@@ -768,6 +873,8 @@
 #endif /* _WIN32 */
 
 
+#if ! defined(HAVE_ARC4RANDOM_BUF) && ! defined(HAVE_ARC4RANDOM)
+
 static unsigned long
 gather_time_entropy(void)
 {
@@ -780,16 +887,20 @@
   int gettimeofday_res;
 
   gettimeofday_res = gettimeofday(&tv, NULL);
+
+#if defined(NDEBUG)
+  (void)gettimeofday_res;
+#else
   assert (gettimeofday_res == 0);
+#endif  /* defined(NDEBUG) */
 
   /* Microseconds time is <20 bits entropy */
   return tv.tv_usec;
 #endif
 }
 
-#if defined(HAVE_ARC4RANDOM_BUF) && defined(HAVE_LIBBSD)
-# include <bsd/stdlib.h>
-#endif
+#endif  /* ! defined(HAVE_ARC4RANDOM_BUF) && ! defined(HAVE_ARC4RANDOM) */
+
 
 static unsigned long
 ENTROPY_DEBUG(const char * label, unsigned long entropy) {
@@ -808,10 +919,12 @@
 {
   unsigned long entropy;
   (void)parser;
-#if defined(HAVE_ARC4RANDOM_BUF) || defined(__CloudABI__)
-  (void)gather_time_entropy;
+#if defined(HAVE_ARC4RANDOM_BUF)
   arc4random_buf(&entropy, sizeof(entropy));
   return ENTROPY_DEBUG("arc4random_buf", entropy);
+#elif defined(HAVE_ARC4RANDOM)
+  writeRandomBytes_arc4random((void *)&entropy, sizeof(entropy));
+  return ENTROPY_DEBUG("arc4random", entropy);
 #else
   /* Try high quality providers first .. */
 #ifdef _WIN32
@@ -819,10 +932,15 @@
     return ENTROPY_DEBUG("RtlGenRandom", entropy);
   }
 #elif defined(HAVE_GETRANDOM) || defined(HAVE_SYSCALL_GETRANDOM)
-  if (writeRandomBytes_getrandom((void *)&entropy, sizeof(entropy))) {
+  if (writeRandomBytes_getrandom_nonblock((void *)&entropy, sizeof(entropy))) {
     return ENTROPY_DEBUG("getrandom", entropy);
   }
 #endif
+#if ! defined(_WIN32) && defined(XML_DEV_URANDOM)
+  if (writeRandomBytes_dev_urandom((void *)&entropy, sizeof(entropy))) {
+    return ENTROPY_DEBUG("/dev/urandom", entropy);
+  }
+#endif  /* ! defined(_WIN32) && defined(XML_DEV_URANDOM) */
   /* .. and self-made low quality for backup: */
 
   /* Process ID is 0 bits entropy if attacker has local access */
@@ -833,7 +951,7 @@
     return ENTROPY_DEBUG("fallback(4)", entropy * 2147483647);
   } else {
     return ENTROPY_DEBUG("fallback(8)",
-        entropy * (unsigned long)2305843009213693951);
+        entropy * (unsigned long)2305843009213693951ULL);
   }
 #endif
 }
@@ -962,6 +1080,8 @@
   nsAttsVersion = 0;
   nsAttsPower = 0;
 
+  protocolEncodingName = NULL;
+
   poolInit(&tempPool, &(parser->m_mem));
   poolInit(&temp2Pool, &(parser->m_mem));
   parserInit(parser, encodingName);
@@ -988,9 +1108,9 @@
 {
   processor = prologInitProcessor;
   XmlPrologStateInit(&prologState);
-  protocolEncodingName = (encodingName != NULL
-                          ? poolCopyString(&tempPool, encodingName)
-                          : NULL);
+  if (encodingName != NULL) {
+    protocolEncodingName = copyString(encodingName, &(parser->m_mem));
+  }
   curBase = NULL;
   XmlInitEncoding(&initEncoding, &encoding, 0);
   userData = NULL;
@@ -1103,6 +1223,8 @@
     unknownEncodingRelease(unknownEncodingData);
   poolClear(&tempPool);
   poolClear(&temp2Pool);
+  FREE((void *)protocolEncodingName);
+  protocolEncodingName = NULL;
   parserInit(parser, encodingName);
   dtdReset(_dtd, &parser->m_mem);
   return XML_TRUE;
@@ -1119,10 +1241,16 @@
   */
   if (ps_parsing == XML_PARSING || ps_parsing == XML_SUSPENDED)
     return XML_STATUS_ERROR;
+
+  /* Get rid of any previous encoding name */
+  FREE((void *)protocolEncodingName);
+
   if (encodingName == NULL)
+    /* No new encoding name */
     protocolEncodingName = NULL;
   else {
-    protocolEncodingName = poolCopyString(&tempPool, encodingName);
+    /* Copy the new encoding name into allocated memory */
+    protocolEncodingName = copyString(encodingName, &(parser->m_mem));
     if (!protocolEncodingName)
       return XML_STATUS_ERROR;
   }
@@ -1357,6 +1485,7 @@
   destroyBindings(inheritedBindings, parser);
   poolDestroy(&tempPool);
   poolDestroy(&temp2Pool);
+  FREE((void *)protocolEncodingName);
 #ifdef XML_DTD
   /* external parameter entity parsers share the DTD structure
      parser->m_dtd with the root parser, so we must not destroy it
@@ -1748,7 +1877,8 @@
 XML_Parse(XML_Parser parser, const char *s, int len, int isFinal)
 {
   if ((parser == NULL) || (len < 0) || ((s == NULL) && (len != 0))) {
-    errorCode = XML_ERROR_INVALID_ARGUMENT;
+    if (parser != NULL)
+      parser->m_errorCode = XML_ERROR_INVALID_ARGUMENT;
     return XML_STATUS_ERROR;
   }
   switch (ps_parsing) {
@@ -1783,9 +1913,22 @@
     if (errorCode == XML_ERROR_NONE) {
       switch (ps_parsing) {
       case XML_SUSPENDED:
+        /* It is hard to be certain, but it seems that this case
+         * cannot occur.  This code is cleaning up a previous parse
+         * with no new data (since len == 0).  Changing the parsing
+         * state requires getting to execute a handler function, and
+         * there doesn't seem to be an opportunity for that while in
+         * this circumstance.
+         *
+         * Given the uncertainty, we retain the code but exclude it
+         * from coverage tests.
+         *
+         * LCOV_EXCL_START
+         */
         XmlUpdatePosition(encoding, positionPtr, bufferPtr, &position);
         positionPtr = bufferPtr;
         return XML_STATUS_SUSPENDED;
+        /* LCOV_EXCL_STOP */
       case XML_INITIALIZED:
       case XML_PARSING:
         ps_parsing = XML_FINISHED;
@@ -2974,9 +3117,17 @@
         return XML_ERROR_NO_MEMORY;
       break;
     default:
+      /* All of the tokens produced by XmlContentTok() have their own
+       * explicit cases, so this default is not strictly necessary.
+       * However it is a useful safety net, so we retain the code and
+       * simply exclude it from the coverage tests.
+       *
+       * LCOV_EXCL_START
+       */
       if (defaultHandler)
         reportDefault(parser, enc, s, next);
       break;
+      /* LCOV_EXCL_STOP */
     }
     *eventPP = s = next;
     switch (ps_parsing) {
@@ -3067,13 +3218,17 @@
 #endif
     attsSize = n + nDefaultAtts + INIT_ATTS_SIZE;
     temp = (ATTRIBUTE *)REALLOC((void *)atts, attsSize * sizeof(ATTRIBUTE));
-    if (temp == NULL)
+    if (temp == NULL) {
+      attsSize = oldAttsSize;
       return XML_ERROR_NO_MEMORY;
+    }
     atts = temp;
 #ifdef XML_ATTR_INFO
     temp2 = (XML_AttrInfo *)REALLOC((void *)attInfo, attsSize * sizeof(XML_AttrInfo));
-    if (temp2 == NULL)
+    if (temp2 == NULL) {
+      attsSize = oldAttsSize;
       return XML_ERROR_NO_MEMORY;
+    }
     attInfo = temp2;
 #endif
     if (n > oldAttsSize)
@@ -3210,6 +3365,7 @@
     int j;  /* hash table index */
     unsigned long version = nsAttsVersion;
     int nsAttsSize = (int)1 << nsAttsPower;
+    unsigned char oldNsAttsPower = nsAttsPower;
     /* size of hash table must be at least 2 * (# of prefixed attributes) */
     if ((nPrefixes << 1) >> nsAttsPower) {  /* true for nsAttsPower = 0 */
       NS_ATT *temp;
@@ -3219,8 +3375,11 @@
         nsAttsPower = 3;
       nsAttsSize = (int)1 << nsAttsPower;
       temp = (NS_ATT *)REALLOC(nsAtts, nsAttsSize * sizeof(NS_ATT));
-      if (!temp)
+      if (!temp) {
+        /* Restore actual size of memory in nsAtts */
+        nsAttsPower = oldNsAttsPower;
         return XML_ERROR_NO_MEMORY;
+      }
       nsAtts = temp;
       version = 0;  /* force re-initialization of nsAtts hash table */
     }
@@ -3247,8 +3406,23 @@
 
         ((XML_Char *)s)[-1] = 0;  /* clear flag */
         id = (ATTRIBUTE_ID *)lookup(parser, &dtd->attributeIds, s, 0);
-        if (!id || !id->prefix)
-          return XML_ERROR_NO_MEMORY;
+        if (!id || !id->prefix) {
+          /* This code is walking through the appAtts array, dealing
+           * with (in this case) a prefixed attribute name.  To be in
+           * the array, the attribute must have already been bound, so
+           * has to have passed through the hash table lookup once
+           * already.  That implies that an entry for it already
+           * exists, so the lookup above will return a pointer to
+           * already allocated memory.  There is no opportunaity for
+           * the allocator to fail, so the condition above cannot be
+           * fulfilled.
+           *
+           * Since it is difficult to be certain that the above
+           * analysis is complete, we retain the test and merely
+           * remove the code from coverage tests.
+           */
+          return XML_ERROR_NO_MEMORY; /* LCOV_EXCL_LINE */
+        }
         b = id->prefix->binding;
         if (!b)
           return XML_ERROR_UNBOUND_PREFIX;
@@ -3625,8 +3799,16 @@
       }
       return XML_ERROR_UNCLOSED_CDATA_SECTION;
     default:
+      /* Every token returned by XmlCdataSectionTok() has its own
+       * explicit case, so this default case will never be executed.
+       * We retain it as a safety net and exclude it from the coverage
+       * statistics.
+       *
+       * LCOV_EXCL_START
+      */
       *eventPP = next;
       return XML_ERROR_UNEXPECTED_STATE;
+      /* LCOV_EXCL_STOP */
     }
 
     *eventPP = s = next;
@@ -3686,8 +3868,20 @@
     eventEndPP = &eventEndPtr;
   }
   else {
+    /* It's not entirely clear, but it seems the following two lines
+     * of code cannot be executed.  The only occasions on which 'enc'
+     * is not 'parser->m_encoding' are when this function is called
+     * from the internal entity processing, and IGNORE sections are an
+     * error in internal entities.
+     *
+     * Since it really isn't clear that this is true, we keep the code
+     * and just remove it from our coverage tests.
+     *
+     * LCOV_EXCL_START
+     */
     eventPP = &(openInternalEntities->internalEventPtr);
     eventEndPP = &(openInternalEntities->internalEventEndPtr);
+    /* LCOV_EXCL_STOP */
   }
   *eventPP = s;
   *startPtr = NULL;
@@ -3720,8 +3914,16 @@
     }
     return XML_ERROR_SYNTAX; /* XML_ERROR_UNCLOSED_IGNORE_SECTION */
   default:
+    /* All of the tokens that XmlIgnoreSectionTok() returns have
+     * explicit cases to handle them, so this default case is never
+     * executed.  We keep it as a safety net anyway, and remove it
+     * from our test coverage statistics.
+     *
+     * LCOV_EXCL_START
+     */
     *eventPP = next;
     return XML_ERROR_UNEXPECTED_STATE;
+    /* LCOV_EXCL_STOP */
   }
   /* not reached */
 }
@@ -3734,6 +3936,7 @@
   const char *s;
 #ifdef XML_UNICODE
   char encodingBuf[128];
+  /* See comments abount `protoclEncodingName` in parserInit() */
   if (!protocolEncodingName)
     s = NULL;
   else {
@@ -3817,7 +4020,14 @@
     reportDefault(parser, encoding, s, next);
   if (protocolEncodingName == NULL) {
     if (newEncoding) {
-      if (newEncoding->minBytesPerChar != encoding->minBytesPerChar) {
+      /* Check that the specified encoding does not conflict with what
+       * the parser has already deduced.  Do we have the same number
+       * of bytes in the smallest representation of a character?  If
+       * this is UTF-16, is it the same endianness?
+       */
+      if (newEncoding->minBytesPerChar != encoding->minBytesPerChar
+          || (newEncoding->minBytesPerChar == 2 &&
+              newEncoding != encoding)) {
         eventPtr = encodingName;
         return XML_ERROR_INCORRECT_ENCODING;
       }
@@ -3962,15 +4172,14 @@
       result = processXmlDecl(parser, 0, start, next);
       if (result != XML_ERROR_NONE)
         return result;
-      switch (ps_parsing) {
-      case XML_SUSPENDED:
-        *nextPtr = next;
-        return XML_ERROR_NONE;
-      case XML_FINISHED:
+      /* At this point, ps_parsing cannot be XML_SUSPENDED.  For that
+       * to happen, a parameter entity parsing handler must have
+       * attempted to suspend the parser, which fails and raises an
+       * error.  The parser can be aborted, but can't be suspended.
+       */
+      if (ps_parsing == XML_FINISHED)
         return XML_ERROR_ABORTED;
-      default:
-        *nextPtr = next;
-      }
+      *nextPtr = next;
       /* stop scanning for text declaration - we found one */
       processor = entityValueProcessor;
       return entityValueProcessor(parser, next, end, nextPtr);
@@ -4293,8 +4502,14 @@
                                             &dtd->paramEntities,
                                             externalSubsetName,
                                             sizeof(ENTITY));
-          if (!entity)
-            return XML_ERROR_NO_MEMORY;
+          if (!entity) {
+            /* The external subset name "#" will have already been
+             * inserted into the hash table at the start of the
+             * external entity parsing, so no allocation will happen
+             * and lookup() cannot fail.
+             */
+            return XML_ERROR_NO_MEMORY; /* LCOV_EXCL_LINE */
+          }
           if (useForeignDTD)
             entity->base = curBase;
           dtd->paramEntityRead = XML_FALSE;
@@ -4773,8 +4988,10 @@
       if (prologState.level >= groupSize) {
         if (groupSize) {
           char *temp = (char *)REALLOC(groupConnector, groupSize *= 2);
-          if (temp == NULL)
+          if (temp == NULL) {
+            groupSize /= 2;
             return XML_ERROR_NO_MEMORY;
+          }
           groupConnector = temp;
           if (dtd->scaffIndex) {
             int *temp = (int *)REALLOC(dtd->scaffIndex,
@@ -4786,8 +5003,10 @@
         }
         else {
           groupConnector = (char *)MALLOC(groupSize = 32);
-          if (!groupConnector)
+          if (!groupConnector) {
+            groupSize = 0;
             return XML_ERROR_NO_MEMORY;
+          }
         }
       }
       groupConnector[prologState.level] = 0;
@@ -4850,8 +5069,29 @@
              : !dtd->hasParamEntityRefs)) {
           if (!entity)
             return XML_ERROR_UNDEFINED_ENTITY;
-          else if (!entity->is_internal)
-            return XML_ERROR_ENTITY_DECLARED_IN_PE;
+          else if (!entity->is_internal) {
+            /* It's hard to exhaustively search the code to be sure,
+             * but there doesn't seem to be a way of executing the
+             * following line.  There are two cases:
+             *
+             * If 'standalone' is false, the DTD must have no
+             * parameter entities or we wouldn't have passed the outer
+             * 'if' statement.  That measn the only entity in the hash
+             * table is the external subset name "#" which cannot be
+             * given as a parameter entity name in XML syntax, so the
+             * lookup must have returned NULL and we don't even reach
+             * the test for an internal entity.
+             *
+             * If 'standalone' is true, it does not seem to be
+             * possible to create entities taking this code path that
+             * are not internal entities, so fail the test above.
+             *
+             * Because this analysis is very uncertain, the code is
+             * being left in place and merely removed from the
+             * coverage test statistics.
+             */
+            return XML_ERROR_ENTITY_DECLARED_IN_PE; /* LCOV_EXCL_LINE */
+          }
         }
         else if (!entity) {
           dtd->keepProcessing = dtd->standalone;
@@ -5323,11 +5563,15 @@
             && (poolLength(pool) == 0 || poolLastChar(pool) == 0x20))
           break;
         n = XmlEncode(n, (ICHAR *)buf);
-        if (!n) {
-          if (enc == encoding)
-            eventPtr = ptr;
-          return XML_ERROR_BAD_CHAR_REF;
-        }
+        /* The XmlEncode() functions can never return 0 here.  That
+         * error return happens if the code point passed in is either
+         * negative or greater than or equal to 0x110000.  The
+         * XmlCharRefNumber() functions will all return a number
+         * strictly less than 0x110000 or a negative value if an error
+         * occurred.  The negative value is intercepted above, so
+         * XmlEncode() is never passed a value it might return an
+         * error for.
+         */
         for (i = 0; i < n; i++) {
           if (!poolAppendChar(pool, buf[i]))
             return XML_ERROR_NO_MEMORY;
@@ -5401,8 +5645,26 @@
           break;
         }
         if (entity->open) {
-          if (enc == encoding)
-            eventPtr = ptr;
+          if (enc == encoding) {
+            /* It does not appear that this line can be executed.
+             *
+             * The "if (entity->open)" check catches recursive entity
+             * definitions.  In order to be called with an open
+             * entity, it must have gone through this code before and
+             * been through the recursive call to
+             * appendAttributeValue() some lines below.  That call
+             * sets the local encoding ("enc") to the parser's
+             * internal encoding (internal_utf8 or internal_utf16),
+             * which can never be the same as the principle encoding.
+             * It doesn't appear there is another code path that gets
+             * here with entity->open being TRUE.
+             *
+             * Since it is not certain that this logic is watertight,
+             * we keep the line and merely exclude it from coverage
+             * tests.
+             */
+            eventPtr = ptr; /* LCOV_EXCL_LINE */
+          }
           return XML_ERROR_RECURSIVE_ENTITY_REF;
         }
         if (entity->notation) {
@@ -5429,9 +5691,21 @@
       }
       break;
     default:
+      /* The only token returned by XmlAttributeValueTok() that does
+       * not have an explicit case here is XML_TOK_PARTIAL_CHAR.
+       * Getting that would require an entity name to contain an
+       * incomplete XML character (e.g. \xE2\x82); however previous
+       * tokenisers will have already recognised and rejected such
+       * names before XmlAttributeValueTok() gets a look-in.  This
+       * default case should be retained as a safety net, but the code
+       * excluded from coverage tests.
+       *
+       * LCOV_EXCL_START
+       */
       if (enc == encoding)
         eventPtr = ptr;
       return XML_ERROR_UNEXPECTED_STATE;
+      /* LCOV_EXCL_STOP */
     }
     ptr = next;
   }
@@ -5564,12 +5838,15 @@
           goto endEntityValue;
         }
         n = XmlEncode(n, (ICHAR *)buf);
-        if (!n) {
-          if (enc == encoding)
-            eventPtr = entityTextPtr;
-          result = XML_ERROR_BAD_CHAR_REF;
-          goto endEntityValue;
-        }
+        /* The XmlEncode() functions can never return 0 here.  That
+         * error return happens if the code point passed in is either
+         * negative or greater than or equal to 0x110000.  The
+         * XmlCharRefNumber() functions will all return a number
+         * strictly less than 0x110000 or a negative value if an error
+         * occurred.  The negative value is intercepted above, so
+         * XmlEncode() is never passed a value it might return an
+         * error for.
+         */
         for (i = 0; i < n; i++) {
           if (pool->end == pool->ptr && !poolGrow(pool)) {
             result = XML_ERROR_NO_MEMORY;
@@ -5590,10 +5867,18 @@
       result = XML_ERROR_INVALID_TOKEN;
       goto endEntityValue;
     default:
+      /* This default case should be unnecessary -- all the tokens
+       * that XmlEntityValueTok() can return have their own explicit
+       * cases -- but should be retained for safety.  We do however
+       * exclude it from the coverage statistics.
+       *
+       * LCOV_EXCL_START
+       */
       if (enc == encoding)
         eventPtr = entityTextPtr;
       result = XML_ERROR_UNEXPECTED_STATE;
       goto endEntityValue;
+      /* LCOV_EXCL_STOP */
     }
     entityTextPtr = next;
   }
@@ -5691,8 +5976,25 @@
       eventEndPP = &eventEndPtr;
     }
     else {
+      /* To get here, two things must be true; the parser must be
+       * using a character encoding that is not the same as the
+       * encoding passed in, and the encoding passed in must need
+       * conversion to the internal format (UTF-8 unless XML_UNICODE
+       * is defined).  The only occasions on which the encoding passed
+       * in is not the same as the parser's encoding are when it is
+       * the internal encoding (e.g. a previously defined parameter
+       * entity, already converted to internal format).  This by
+       * definition doesn't need conversion, so the whole branch never
+       * gets executed.
+       *
+       * For safety's sake we don't delete these lines and merely
+       * exclude them from coverage statistics.
+       *
+       * LCOV_EXCL_START
+       */
       eventPP = &(openInternalEntities->internalEventPtr);
       eventEndPP = &(openInternalEntities->internalEventEndPtr);
+      /* LCOV_EXCL_STOP */
     }
     do {
       ICHAR *dataPtr = (ICHAR *)dataBuf;
@@ -5861,9 +6163,30 @@
     len = dtd->defaultPrefix.binding->uriLen;
     if (namespaceSeparator)
       len--;
-    for (i = 0; i < len; i++)
-      if (!poolAppendChar(&tempPool, dtd->defaultPrefix.binding->uri[i]))
-        return NULL;
+    for (i = 0; i < len; i++) {
+      if (!poolAppendChar(&tempPool, dtd->defaultPrefix.binding->uri[i])) {
+        /* Because of memory caching, I don't believe this line can be
+         * executed.
+         *
+         * This is part of a loop copying the default prefix binding
+         * URI into the parser's temporary string pool.  Previously,
+         * that URI was copied into the same string pool, with a
+         * terminating NUL character, as part of setContext().  When
+         * the pool was cleared, that leaves a block definitely big
+         * enough to hold the URI on the free block list of the pool.
+         * The URI copy in getContext() therefore cannot run out of
+         * memory.
+         *
+         * If the pool is used between the setContext() and
+         * getContext() calls, the worst it can do is leave a bigger
+         * block on the front of the free list.  Given that this is
+         * all somewhat inobvious and program logic can be changed, we
+         * don't delete the line but we do exclude it from the test
+         * coverage statistics.
+         */
+        return NULL; /* LCOV_EXCL_LINE */
+      }
+    }
     needSep = XML_TRUE;
   }
 
@@ -5875,8 +6198,15 @@
     PREFIX *prefix = (PREFIX *)hashTableIterNext(&iter);
     if (!prefix)
       break;
-    if (!prefix->binding)
-      continue;
+    if (!prefix->binding) {
+      /* This test appears to be (justifiable) paranoia.  There does
+       * not seem to be a way of injecting a prefix without a binding
+       * that doesn't get errored long before this function is called.
+       * The test should remain for safety's sake, so we instead
+       * exclude the following line from the coverage statistics.
+       */
+      continue; /* LCOV_EXCL_LINE */
+    }
     if (needSep && !poolAppendChar(&tempPool, CONTEXT_SEP))
       return NULL;
     for (s = prefix->name; *s; s++)
@@ -6547,8 +6877,20 @@
 static const XML_Char *
 poolCopyStringN(STRING_POOL *pool, const XML_Char *s, int n)
 {
-  if (!pool->ptr && !poolGrow(pool))
-    return NULL;
+  if (!pool->ptr && !poolGrow(pool)) {
+    /* The following line is unreachable given the current usage of
+     * poolCopyStringN().  Currently it is called from exactly one
+     * place to copy the text of a simple general entity.  By that
+     * point, the name of the entity is already stored in the pool, so
+     * pool->ptr cannot be NULL.
+     *
+     * If poolCopyStringN() is used elsewhere as it well might be,
+     * this line may well become executable again.  Regardless, this
+     * sort of check shouldn't be removed lightly, so we just exclude
+     * it from the coverage statistics.
+     */
+    return NULL; /* LCOV_EXCL_LINE */
+  }
   for (; n > 0; --n, s++) {
     if (!poolAppendChar(pool, *s))
       return NULL;
@@ -6641,8 +6983,19 @@
     int blockSize = (int)((unsigned)(pool->end - pool->start)*2U);
     size_t bytesToAllocate;
 
-    if (blockSize < 0)
-      return XML_FALSE;
+    // NOTE: Needs to be calculated prior to calling `realloc`
+    //       to avoid dangling pointers:
+    const ptrdiff_t offsetInsideBlock = pool->ptr - pool->start;
+
+    if (blockSize < 0) {
+      /* This condition traps a situation where either more than
+       * INT_MAX/2 bytes have already been allocated.  This isn't
+       * readily testable, since it is unlikely that an average
+       * machine will have that much memory, so we exclude it from the
+       * coverage statistics.
+       */
+      return XML_FALSE; /* LCOV_EXCL_LINE */
+    }
 
     bytesToAllocate = poolBytesToAllocateFor(blockSize);
     if (bytesToAllocate == 0)
@@ -6654,7 +7007,7 @@
       return XML_FALSE;
     pool->blocks = temp;
     pool->blocks->size = blockSize;
-    pool->ptr = pool->blocks->s + (pool->ptr - pool->start);
+    pool->ptr = pool->blocks->s + offsetInsideBlock;
     pool->start = pool->blocks->s;
     pool->end = pool->start + blockSize;
   }
@@ -6663,8 +7016,18 @@
     int blockSize = (int)(pool->end - pool->start);
     size_t bytesToAllocate;
 
-    if (blockSize < 0)
-      return XML_FALSE;
+    if (blockSize < 0) {
+      /* This condition traps a situation where either more than
+       * INT_MAX bytes have already been allocated (which is prevented
+       * by various pieces of program logic, not least this one, never
+       * mind the unlikelihood of actually having that much memory) or
+       * the pool control fields have been corrupted (which could
+       * conceivably happen in an extremely buggy user handler
+       * function).  Either way it isn't readily testable, so we
+       * exclude it from the coverage statistics.
+       */
+      return XML_FALSE;  /* LCOV_EXCL_LINE */
+    }
 
     if (blockSize < INIT_BLOCK_SIZE)
       blockSize = INIT_BLOCK_SIZE;
@@ -6827,3 +7190,26 @@
   }
   return ret;
 }
+
+static XML_Char *
+copyString(const XML_Char *s,
+           const XML_Memory_Handling_Suite *memsuite)
+{
+    int charsRequired = 0;
+    XML_Char *result;
+
+    /* First determine how long the string is */
+    while (s[charsRequired] != 0) {
+      charsRequired++;
+    }
+    /* Include the terminator */
+    charsRequired++;
+
+    /* Now allocate space for the copy */
+    result = memsuite->malloc_fcn(charsRequired * sizeof(XML_Char));
+    if (result == NULL)
+        return NULL;
+    /* Copy the original into place */
+    memcpy(result, s, charsRequired * sizeof(XML_Char));
+    return result;
+}