s390guy
The Source for gcccsect is missing.
"Fix the unlabeled CSECT produced by the GCC compiler", sounds like a missing feature of our gcc implementation.
I think its a PITA to fix. I think it used to be labelled but it broke the VM loader. What issues does this cause?
System380.txt says: "[...] One of the bugs hit early on was the fact that the compiler was converting static functions into 8-character assembler identifiers according to their actual name, which mean that they needed to be unique within the source file. When the dust settled, there were about 3000 functions that had to be #defined to new names, about half of them static (C90 requires the statics to be unique to more than 8 characters, so it was the MVS port of GCC that was at fault). To make matters worse, the code was initially generating CSECTs for each function name. The IBM linker is designed to just take the largest CSECT of the same name and silently use that instead of reporting a name clash. The code generator was changed to use unnamed CSECTs and use ENTRY statements instead to ensure external identifiers were unique and clashes detected. Years later, the static bug was fixed and a tool developed to search out duplicates in the generated assembler so as to only keep those names that needed to be kept (ie external name clashes, which ended up being about 1300). Although the GNU config.h is annoying in that they don't provide a C90 version by default, and instead one needs to be constructed manually, it does have the advantage that all the remaps were able to be done in there and get picked up across the entire source base."
we don't use the IBM linker on VM we use the LOADER. I can't find any trace of the name de-duplicator.
The problem I was investigating when I asked where this source was is https://github.com/adesutherland/CMS-370-BREXX/issues/88. I finally had this occur a few days ago, on my local VM/CE 1.1.2 system, and since once it occurs, it happens repeatedly, I was able to figure that out.
In the process, I learned that the source Rene located is not exactly the code in GCCCSECT MODULE Y on VM/CE 1.1.2. Both have bugs, and the bug in the code I disassembled and decompiled from the Y-disk MODULE causes the failure I saw when compiling bREXX. The bug in the code Rene found is simpler - it's that the CSECT label may match another label, because 8-character filenames will not be modified to end with an @.
Here's the diff between the two versions. The root cause of the bREXX compilation failure is the strncpy(csectName, argv[1], 7); in line 57, which can leave the csectName string unterminated if the filename (argv[1]) is 7 or 8 characters long (hence the failures I'm seeing on BUILTIN C, CHANGEST C, etc.). Whether or not the string is terminated will depend upon the pre-existing value of csectName[7], which also explains why the only thing that seems to stop this failure from repeating is to wipe storage (e.g., LOGOFF or IPL CMS). This is especially true in the GitHub bREXX builds, where the instruction stream executed by the CMS user can be identical from run to run.
--- gcccsect.c_issue.a 1970-01-01 00:00:00.000000000
+++ gcccsect.c_module.a 1970-01-01 00:00:00.000000000
@@ -26,14 +26,14 @@
int recnum;
if (argc != 4) {
- CMSconsoleWrite("Usage is GCCCSECT fn ft fm", CMS_EDIT);
+ CMSconsoleWrite("Usage is GCCCSECT fn ft fm\x15", CMS_EDIT);
return 24;
}
// First build the fileid from the argument list.
-strcpy(readFileid, argv[1]); strncat(readFileid, " ", 8 - strlen(readFileid));
+strncpy(readFileid, argv[1], 8); strncat(readFileid, " ", 8 - strlen(readFileid));
-strcat(readFileid, argv[2]); strncat(readFileid, " ", 16 - strlen(readFileid));
+strncat(readFileid, argv[2], 8); strncat(readFileid, " ", 16 - strlen(readFileid));
-strcat(readFileid, argv[3]); strncat(readFileid, " ", 18 - strlen(readFileid));
+strncat(readFileid, argv[3], 2); strncat(readFileid, " ", 18 - strlen(readFileid));
readFileid[18] = 0;
for (i = 0; i < 18; i++) { // now convert the fileid to uppercase
readFileid[i] = toupper(readFileid[i]);
@@ -44,17 +44,17 @@
// Open the input and output files.
rc = CMSfileOpen(readFileid, buffer, RECLEN, 'V', 1, 1, &readFscb);
if (rc != 0) {
- CMSconsoleWrite("Unable to open input file.", CMS_EDIT);
+ CMSconsoleWrite("Unable to open input file.\x15", CMS_EDIT);
return rc;
}
rc = CMSfileOpen(writeFileid, buffer, RECLEN, 'F', 1, 1, &writeFscb);
if (!(rc == 0 || rc == 28)) {
- CMSconsoleWrite("Unable to open output file.", CMS_EDIT);
+ CMSconsoleWrite("Unable to open output file.\x15", CMS_EDIT);
return rc;
}
// Commence processing in earnest. Loop to search for the CSECT record, and add a label.
-strcpy(csectName, argv[1]);
+strncpy(csectName, argv[1], 7);
strncat(csectName, "@@@@@@@@", 8 - strlen(csectName));
for (i = 0; i < 8; i++) csectName[i] = toupper(csectName[i]);
notyet = 1;
@@ -70,13 +70,13 @@
rc = CMSfileWrite(&writeFscb, recnum, RECLEN); // write the record to the output file
recnum = 0;
if (rc > 0) {
- CMSconsoleWrite("Error writing output file.", CMS_EDIT);
+ CMSconsoleWrite("Error writing output file.\x15", CMS_EDIT);
break;
}
rc = CMSfileRead(&readFscb, 0, &bytesRead);
}
if (rc == 12) rc = 0; // rc of 12 means end of file, and all is well
-else CMSconsoleWrite("Error reading input file.", CMS_EDIT);
+else CMSconsoleWrite("Error reading input file.\x15", CMS_EDIT);
CMSfileClose(&readFscb);
CMSfileClose(&writeFscb);
return rc;
The GCCCSECT code appears to have two purposes. First, it pads its input records (the GCC output assembler code file) with blanks to column 80. Second, it finds the first CSECT record and renames it.
The first purpose is unnecessary - at least as of VM/CE 1.1.2, GCC creates an F/80 output file.
The second purpose is necessary, but not completely clear. The intended result appears to be that the CSECT name is up to the first 7 characters of the input filename, padded to a length of 8 with '@' characters. I believe this technique is intended to reduce the probability of name collisions that I mentioned above. Certainly, the NAMCSECT program supplied on GCCCMS 291 (and compiled to NAMCSECT MODULE Y) doesn't do that - it just uses the filename - so maybe that's the reason for not using NAMCSECT.
The fix for this appears to be trivial - terminate the csectName string after copying from the filename. I'll be testing that over the next few days.
--- gcccsect.c_module.e¦1970-01-01 00:00:00.000000000
+++ gcccsect.c.e¦1970-01-01 00:00:00.000000000
@@ -54,7 +54,7 @@
}
// Commence processing in earnest. Loop to search for the CSECT record, and ad
-strncpy(csectName, argv[1], 7);
+strcpy(csectName, argv[1]); csectName[7] = '\0';
strncat(csectName, "@@@@@@@@", 8 - strlen(csectName));
for (i = 0; i < 8; i++) csectName[i] = toupper(csectName[i]);
notyet = 1;
By the way you may use \n. https://github.com/adesutherland/CMS-370-GCCLIB/blob/8babe4674df8206e8666af3a991b8c0a166fe32a/manual.md?plain=1#L293
Ross Patterson wrote:
[...]
The root cause of the bREXX compilation failure is the
strncpy(csectName, argv[1], 7);in line 57, which can leave the csectName string unterminated if the filename (argv[1]) is 7 or 8 characters long (hence the failures I'm seeing on BUILTIN C, CHANGEST C, etc.).
As a C/C++ programmer for many years, I would personally STRONGLY recommend avoiding the use of 'strcpy', 'strcat' and similar functions AT ALL COSTS. (Same goes with 'sprintf' and 'snprintf' too.)
ALL such functions are the most frequent cause of buffer overflows, the bane of security experts the world over.
Instead, you should be using something like Hercules's STRLCPY, STRLCAT and MSGBUF macros.
Just saying.
-- "Fish" (David B. Trout) Software Development Laboratories http://www.softdevlabs.com mail: @.***
As a C/C++ programmer for many years, I would personally STRONGLY recommend avoiding the use of 'strcpy', 'strcat' and similar functions AT ALL COSTS. (Same goes with 'sprintf' and 'snprintf' too.)
I agree completely. I find it slightly humorous that the bug was caused by the use of strncpy(), which was originally introduced to prevent bugs caused by using strcpy() 😀
Instead, you should be using something like Hercules's STRLCPY, STRLCAT and MSGBUF macros.
Yup.
Here is the source for the updated-to-match-the-MODULE and bug-fixed version I'm using in compiling bREXX.